Basic StarFive JH7100 RISC-V SoC support
Subsystems (16)
Emil Renner Berthing
maintainerv3 cover Basic StarFive JH7100 RISC-V SoC support
This series adds support for the StarFive JH7100 RISC-V SoC. The SoC has many devices that need non-coherent dma operations to work which isn't upstream yet[1], so this just adds basic support to boot up, get a serial console, blink an LED and reboot itself. Unlike the Allwinner D1 this chip doesn't use any extra pagetable bits, but instead the DDR RAM appears twice in the memory map, with and without the cache.
The JH7100 is a test chip for the upcoming JH7110 and about 300 BeagleV Starlight Beta boards were sent out with them as part of a now cancelled BeagleBoard.org project. However StarFive has produced more of the JH7100s and more boards will be available[2] to buy. I've seen pictures of the new boards now, so hopefully before the end of the year.
This series is also available at
https://github.com/esmil/linux/commits/starlight-minimal
..but a more complete kernel including drivers for non-coherent peripherals based on this series can be found at
https://github.com/starfive-tech/linux/tree/visionfive
[1]: https://lore.kernel.org/linux-riscv/[email protected]/
[2]: https://www.linkedin.com/pulse/starfive-release-open-source-single-board-platform-q3-2021-starfive/
/Emil
Changes since v2:
- Ahmad and Geert agreed to switch the license of the clock and reset dt headers to GPL-2.0 OR MIT, so that both headers and device tree files can all use the same license.
Bindings are still GPL-2.0-only OR BSD-2-Clause as recommended.
- Clock and reset drivers now set .suppress_bind_attrs = true and use builtin_platform_driver_probe to make sure the probe function is only called at init time so we can use __init and __initconst.
- The clock driver now uses devm_clk_hw_register and .parent_data when registering clocks. This way we can use the dt clock indexes rather than strings for parent lists and decrease the amount of static data needed considerably.
- Various dt binding cleanups from Rob
- Reworked description in the pinctrl dt binding.
- Pinctrl driver now depends on CONFIG_OF again since it uses pinconf_generic_parse_dt_config which is otherwise not defined.
- Pinctrl no longer devm_kfree's data that won't be referenced if dt pinconf parsing fails before registering groups and function, and other nits by Andy.
- The dw8250 quirk no longer needs a skip_clk_set_rate bit, but sets port->set_termios to the function called after clk_set_rate.
Changes since v1:
- Let SOC_STARFIVE select RESET_CONTROLLER but drop SERIAL_8250_DW
- Add missing Signed-of-by to clock dt-binding header
- Use builtin_platform_driver macro for the clock driver, add explicit comment to the determine_rate callback and other small nits from Andy
- Use reset-controller for node names in documentation and device tree
- Use readl_poll_timeout in reset driver to avoid hanging forever if a driver leaves the associated clock gated and sort Kconfig and Makefile entries properly.
- In the pinctrl driver align register names with documentation, remove invalid __init tag from probe function, use of_property_* functions to parse device tree, hoist pinmux unpacking into helper function to better document what's going on, bail on invalid signal group in device tree and fix many other nits from Andy.
- Refactor and rebase 8250_dw quirk on tty-next
Emil Renner Berthing (12):
RISC-V: Add StarFive SoC Kconfig option
dt-bindings: timer: Add StarFive JH7100 clint
dt-bindings: interrupt-controller: Add StarFive JH7100 plic
dt-bindings: reset: Add Starfive JH7100 reset bindings
reset: starfive-jh7100: Add StarFive JH7100 reset driver
dt-bindings: pinctrl: Add StarFive pinctrl definitions
dt-bindings: pinctrl: Add StarFive JH7100 bindings
pinctrl: starfive: Add pinctrl driver for StarFive SoCs
dt-bindings: serial: snps-dw-apb-uart: Add JH7100 uarts
serial: 8250_dw: Add StarFive JH7100 quirk
RISC-V: Add initial StarFive JH7100 device tree
RISC-V: Add BeagleV Starlight Beta device tree
Geert Uytterhoeven (4):
dt-bindings: clock: starfive: Add JH7100 clock definitions
dt-bindings: clock: starfive: Add JH7100 bindings
clk: starfive: Add JH7100 clock generator driver
dt-bindings: reset: Add StarFive JH7100 reset definitions
.../clock/starfive,jh7100-clkgen.yaml | 56 + .../sifive,plic-1.0.0.yaml | 1 + .../pinctrl/starfive,jh7100-pinctrl.yaml | 307 ++++ .../bindings/reset/starfive,jh7100-reset.yaml | 38 + .../bindings/serial/snps-dw-apb-uart.yaml | 5 + .../bindings/timer/sifive,clint.yaml | 1 + MAINTAINERS | 22 + arch/riscv/Kconfig.socs | 8 + arch/riscv/boot/dts/Makefile | 1 + arch/riscv/boot/dts/starfive/Makefile | 2 + .../dts/starfive/jh7100-beaglev-starlight.dts | 164 ++ arch/riscv/boot/dts/starfive/jh7100.dtsi | 230 +++ drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 1 + drivers/clk/starfive/Kconfig | 9 + drivers/clk/starfive/Makefile | 3 + drivers/clk/starfive/clk-starfive-jh7100.c | 689 +++++++++ drivers/pinctrl/Kconfig | 17 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-starfive.c | 1353 +++++++++++++++++ drivers/reset/Kconfig | 7 + drivers/reset/Makefile | 1 + drivers/reset/reset-starfive-jh7100.c | 178 +++ drivers/tty/serial/8250/8250_dw.c | 3 + include/dt-bindings/clock/starfive-jh7100.h | 202 +++ .../dt-bindings/pinctrl/pinctrl-starfive.h | 275 ++++ include/dt-bindings/reset/starfive-jh7100.h | 126 ++ 27 files changed, 3701 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml create mode 100644 Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml create mode 100644 arch/riscv/boot/dts/starfive/Makefile create mode 100644 arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts create mode 100644 arch/riscv/boot/dts/starfive/jh7100.dtsi create mode 100644 drivers/clk/starfive/Kconfig create mode 100644 drivers/clk/starfive/Makefile create mode 100644 drivers/clk/starfive/clk-starfive-jh7100.c create mode 100644 drivers/pinctrl/pinctrl-starfive.c create mode 100644 drivers/reset/reset-starfive-jh7100.c create mode 100644 include/dt-bindings/clock/starfive-jh7100.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive.h create mode 100644 include/dt-bindings/reset/starfive-jh7100.h
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 1/16 RISC-V: Add StarFive SoC Kconfig option
Add StarFive Kconfig option to select SoC specific and common drivers required for these SoCs. Select subsystems required to boot so the required drivers gets enabled by default.
--- arch/riscv/Kconfig.socs | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/riscv/Kconfig.socs +8 -0
@@ -19,6 +19,14 @@ config SOC_SIFIVE help This enables support for SiFive SoC platform hardware. +config SOC_STARFIVE + bool "StarFive SoCs" + select PINCTRL + select RESET_CONTROLLER + select SIFIVE_PLIC + help + This enables support for StarFive SoC platform hardware. + config SOC_VIRT bool "QEMU Virt Machine" select CLINT_TIMER if RISCV_M_MODE
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 01/16] RISC-V: Add StarFive SoC Kconfig option
Quoted 5 lines · Emil Renner Berthing
Add StarFive
Kconfigoption to select SoC specific and common drivers required for these SoCs. Select subsystems required to boot so the required drivers gets enabled by default.Signed-off-by Emil Renner Berthing <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Emil Renner Berthing
maintainerPATCH v3 2/16 dt-bindings: timer: Add StarFive JH7100 clint
In reply to Emil Renner BerthingAdd compatible string for the StarFive JH7100 clint.
--- Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/timer/sifive,clint.yaml +1 -0
@@ -25,6 +25,7 @@ properties: items: - enum: - sifive,fu540-c000-clint + - starfive,jh7100-clint - canaan,k210-clint - const: sifive,clint0
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 3/16 dt-bindings: interrupt-controller: Add StarFive JH7100 plic
In reply to Emil Renner BerthingAdd compatible string for StarFive JH7100 plic.
--- .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml +1 -0
@@ -45,6 +45,7 @@ properties: items: - enum: - sifive,fu540-c000-plic + - starfive,jh7100-plic - canaan,k210-plic - const: sifive,plic-1.0.0
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 4/16 dt-bindings: clock: starfive: Add JH7100 clock definitions
In reply to Emil Renner BerthingFrom: Geert Uytterhoeven <[email protected]>
Add all clock outputs for the StarFive JH7100 clock generator.
Based on work by Ahmad Fatoum for Barebox, with "JH7100_" prefixes added to all definitions.
--- include/dt-bindings/clock/starfive-jh7100.h | 202 ++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 include/dt-bindings/clock/starfive-jh7100.h
include/dt-bindings/clock/starfive-jh7100.h new file +202 -0
@@ -0,0 +1,202 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ +/* +* Copyright (C) 2021 Ahmad Fatoum, Pengutronix +*/ + +#ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7100_H__ +#define __DT_BINDINGS_CLOCK_STARFIVE_JH7100_H__ + +#define JH7100_CLK_CPUNDBUS_ROOT 0 +#define JH7100_CLK_DLA_ROOT 1 +#define JH7100_CLK_DSP_ROOT 2 +#define JH7100_CLK_GMACUSB_ROOT 3 +#define JH7100_CLK_PERH0_ROOT 4 +#define JH7100_CLK_PERH1_ROOT 5 +#define JH7100_CLK_VIN_ROOT 6 +#define JH7100_CLK_VOUT_ROOT 7 +#define JH7100_CLK_AUDIO_ROOT 8 +#define JH7100_CLK_CDECHIFI4_ROOT 9 +#define JH7100_CLK_CDEC_ROOT 10 +#define JH7100_CLK_VOUTBUS_ROOT 11 +#define JH7100_CLK_CPUNBUS_ROOT_DIV 12 +#define JH7100_CLK_DSP_ROOT_DIV 13 +#define JH7100_CLK_PERH0_SRC 14 +#define JH7100_CLK_PERH1_SRC 15 +#define JH7100_CLK_PLL0_TESTOUT 16 +#define JH7100_CLK_PLL1_TESTOUT 17 +#define JH7100_CLK_PLL2_TESTOUT 18 +#define JH7100_CLK_PLL2_REF 19 +#define JH7100_CLK_CPU_CORE 20 +#define JH7100_CLK_CPU_AXI 21 +#define JH7100_CLK_AHB_BUS 22 +#define JH7100_CLK_APB1_BUS 23 +#define JH7100_CLK_APB2_BUS 24 +#define JH7100_CLK_DOM3AHB_BUS 25 +#define JH7100_CLK_DOM7AHB_BUS 26 +#define JH7100_CLK_U74_CORE0 27 +#define JH7100_CLK_U74_CORE1 28 +#define JH7100_CLK_U74_AXI 29 +#define JH7100_CLK_U74RTC_TOGGLE 30 +#define JH7100_CLK_SGDMA2P_AXI 31 +#define JH7100_CLK_DMA2PNOC_AXI 32 +#define JH7100_CLK_SGDMA2P_AHB 33 +#define JH7100_CLK_DLA_BUS 34 +#define JH7100_CLK_DLA_AXI 35 +#define JH7100_CLK_DLANOC_AXI 36 +#define JH7100_CLK_DLA_APB 37 +#define JH7100_CLK_VP6_CORE 38 +#define JH7100_CLK_VP6BUS_SRC 39 +#define JH7100_CLK_VP6_AXI 40 +#define JH7100_CLK_VCDECBUS_SRC 41 +#define JH7100_CLK_VDEC_BUS 42 +#define JH7100_CLK_VDEC_AXI 43 +#define JH7100_CLK_VDECBRG_MAIN 44 +#define JH7100_CLK_VDEC_BCLK 45 +#define JH7100_CLK_VDEC_CCLK 46 +#define JH7100_CLK_VDEC_APB 47 +#define JH7100_CLK_JPEG_AXI 48 +#define JH7100_CLK_JPEG_CCLK 49 +#define JH7100_CLK_JPEG_APB 50 +#define JH7100_CLK_GC300_2X 51 +#define JH7100_CLK_GC300_AHB 52 +#define JH7100_CLK_JPCGC300_AXIBUS 53 +#define JH7100_CLK_GC300_AXI 54 +#define JH7100_CLK_JPCGC300_MAIN 55 +#define JH7100_CLK_VENC_BUS 56 +#define JH7100_CLK_VENC_AXI 57 +#define JH7100_CLK_VENCBRG_MAIN 58 +#define JH7100_CLK_VENC_BCLK 59 +#define JH7100_CLK_VENC_CCLK 60 +#define JH7100_CLK_VENC_APB 61 +#define JH7100_CLK_DDRPLL_DIV2 62 +#define JH7100_CLK_DDRPLL_DIV4 63 +#define JH7100_CLK_DDRPLL_DIV8 64 +#define JH7100_CLK_DDROSC_DIV2 65 +#define JH7100_CLK_DDRC0 66 +#define JH7100_CLK_DDRC1 67 +#define JH7100_CLK_DDRPHY_APB 68 +#define JH7100_CLK_NOC_ROB 69 +#define JH7100_CLK_NOC_COG 70 +#define JH7100_CLK_NNE_AHB 71 +#define JH7100_CLK_NNEBUS_SRC1 72 +#define JH7100_CLK_NNE_BUS 73 +#define JH7100_CLK_NNE_AXI 74 +#define JH7100_CLK_NNENOC_AXI 75 +#define JH7100_CLK_DLASLV_AXI 76 +#define JH7100_CLK_DSPX2C_AXI 77 +#define JH7100_CLK_HIFI4_SRC 78 +#define JH7100_CLK_HIFI4_COREFREE 79 +#define JH7100_CLK_HIFI4_CORE 80 +#define JH7100_CLK_HIFI4_BUS 81 +#define JH7100_CLK_HIFI4_AXI 82 +#define JH7100_CLK_HIFI4NOC_AXI 83 +#define JH7100_CLK_SGDMA1P_BUS 84 +#define JH7100_CLK_SGDMA1P_AXI 85 +#define JH7100_CLK_DMA1P_AXI 86 +#define JH7100_CLK_X2C_AXI 87 +#define JH7100_CLK_USB_BUS 88 +#define JH7100_CLK_USB_AXI 89 +#define JH7100_CLK_USBNOC_AXI 90 +#define JH7100_CLK_USBPHY_ROOTDIV 91 +#define JH7100_CLK_USBPHY_125M 92 +#define JH7100_CLK_USBPHY_PLLDIV25M 93 +#define JH7100_CLK_USBPHY_25M 94 +#define JH7100_CLK_AUDIO_DIV 95 +#define JH7100_CLK_AUDIO_SRC 96 +#define JH7100_CLK_AUDIO_12288 97 +#define JH7100_CLK_VIN_SRC 98 +#define JH7100_CLK_ISP0_BUS 99 +#define JH7100_CLK_ISP0_AXI 100 +#define JH7100_CLK_ISP0NOC_AXI 101 +#define JH7100_CLK_ISPSLV_AXI 102 +#define JH7100_CLK_ISP1_BUS 103 +#define JH7100_CLK_ISP1_AXI 104 +#define JH7100_CLK_ISP1NOC_AXI 105 +#define JH7100_CLK_VIN_BUS 106 +#define JH7100_CLK_VIN_AXI 107 +#define JH7100_CLK_VINNOC_AXI 108 +#define JH7100_CLK_VOUT_SRC 109 +#define JH7100_CLK_DISPBUS_SRC 110 +#define JH7100_CLK_DISP_BUS 111 +#define JH7100_CLK_DISP_AXI 112 +#define JH7100_CLK_DISPNOC_AXI 113 +#define JH7100_CLK_SDIO0_AHB 114 +#define JH7100_CLK_SDIO0_CCLKINT 115 +#define JH7100_CLK_SDIO0_CCLKINT_INV 116 +#define JH7100_CLK_SDIO1_AHB 117 +#define JH7100_CLK_SDIO1_CCLKINT 118 +#define JH7100_CLK_SDIO1_CCLKINT_INV 119 +#define JH7100_CLK_GMAC_AHB 120 +#define JH7100_CLK_GMAC_ROOT_DIV 121 +#define JH7100_CLK_GMAC_PTP_REF 122 +#define JH7100_CLK_GMAC_GTX 123 +#define JH7100_CLK_GMAC_RMII_TX 124 +#define JH7100_CLK_GMAC_RMII_RX 125 +#define JH7100_CLK_GMAC_TX 126 +#define JH7100_CLK_GMAC_TX_INV 127 +#define JH7100_CLK_GMAC_RX_PRE 128 +#define JH7100_CLK_GMAC_RX_INV 129 +#define JH7100_CLK_GMAC_RMII 130 +#define JH7100_CLK_GMAC_TOPHYREF 131 +#define JH7100_CLK_SPI2AHB_AHB 132 +#define JH7100_CLK_SPI2AHB_CORE 133 +#define JH7100_CLK_EZMASTER_AHB 134 +#define JH7100_CLK_E24_AHB 135 +#define JH7100_CLK_E24RTC_TOGGLE 136 +#define JH7100_CLK_QSPI_AHB 137 +#define JH7100_CLK_QSPI_APB 138 +#define JH7100_CLK_QSPI_REF 139 +#define JH7100_CLK_SEC_AHB 140 +#define JH7100_CLK_AES 141 +#define JH7100_CLK_SHA 142 +#define JH7100_CLK_PKA 143 +#define JH7100_CLK_TRNG_APB 144 +#define JH7100_CLK_OTP_APB 145 +#define JH7100_CLK_UART0_APB 146 +#define JH7100_CLK_UART0_CORE 147 +#define JH7100_CLK_UART1_APB 148 +#define JH7100_CLK_UART1_CORE 149 +#define JH7100_CLK_SPI0_APB 150 +#define JH7100_CLK_SPI0_CORE 151 +#define JH7100_CLK_SPI1_APB 152 +#define JH7100_CLK_SPI1_CORE 153 +#define JH7100_CLK_I2C0_APB 154 +#define JH7100_CLK_I2C0_CORE 155 +#define JH7100_CLK_I2C1_APB 156 +#define JH7100_CLK_I2C1_CORE 157 +#define JH7100_CLK_GPIO_APB 158 +#define JH7100_CLK_UART2_APB 159 +#define JH7100_CLK_UART2_CORE 160 +#define JH7100_CLK_UART3_APB 161 +#define JH7100_CLK_UART3_CORE 162 +#define JH7100_CLK_SPI2_APB 163 +#define JH7100_CLK_SPI2_CORE 164 +#define JH7100_CLK_SPI3_APB 165 +#define JH7100_CLK_SPI3_CORE 166 +#define JH7100_CLK_I2C2_APB 167 +#define JH7100_CLK_I2C2_CORE 168 +#define JH7100_CLK_I2C3_APB 169 +#define JH7100_CLK_I2C3_CORE 170 +#define JH7100_CLK_WDTIMER_APB 171 +#define JH7100_CLK_WDT_CORE 172 +#define JH7100_CLK_TIMER0_CORE 173 +#define JH7100_CLK_TIMER1_CORE 174 +#define JH7100_CLK_TIMER2_CORE 175 +#define JH7100_CLK_TIMER3_CORE 176 +#define JH7100_CLK_TIMER4_CORE 177 +#define JH7100_CLK_TIMER5_CORE 178 +#define JH7100_CLK_TIMER6_CORE 179 +#define JH7100_CLK_VP6INTC_APB 180 +#define JH7100_CLK_PWM_APB 181 +#define JH7100_CLK_MSI_APB 182 +#define JH7100_CLK_TEMP_APB 183 +#define JH7100_CLK_TEMP_SENSE 184 +#define JH7100_CLK_SYSERR_APB 185 + +#define JH7100_CLK_PLL0_OUT 186 +#define JH7100_CLK_PLL1_OUT 187 +#define JH7100_CLK_PLL2_OUT 188 + +#define JH7100_CLK_END 189 + +#endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH7100_H__ */
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 5/16 dt-bindings: clock: starfive: Add JH7100 bindings
In reply to Emil Renner BerthingFrom: Geert Uytterhoeven <[email protected]>
Add bindings for the clock generator on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
--- .../clock/starfive,jh7100-clkgen.yaml | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml
Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml new file +56 -0
@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/starfive,jh7100-clkgen.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7100 Clock Generator + +maintainers: + - Geert Uytterhoeven <[email protected]> + - Emil Renner Berthing <[email protected]> + +properties: + compatible: + const: starfive,jh7100-clkgen + + reg: + maxItems: 1 + + clocks: + items: + - description: Main clock source (25 MHz) + - description: Application-specific clock source (12-27 MHz) + - description: RMII reference clock (50 MHz) + - description: RGMII RX clock (125 MHz) + + clock-names: + items: + - const: osc_sys + - const: osc_aud + - const: gmac_rmii_ref + - const: gmac_gr_mii_rxclk + + '#clock-cells': + const: 1 + description: + See <dt-bindings/clock/starfive-jh7100.h> for valid indices. + +required: + - compatible + - reg + - clocks + - clock-names + - '#clock-cells' + +additionalProperties: false + +examples: + - | + clock-controller@11800000 { + compatible = "starfive,jh7100-clkgen"; + reg = <0x11800000 0x10000>; + clocks = <&osc_sys>, <&osc_aud>, <&gmac_rmii_ref>, <&gmac_gr_mii_rxclk>; + clock-names = "osc_sys", "osc_aud", "gmac_rmii_ref", "gmac_gr_mii_rxclk"; + #clock-cells = <1>; + };
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 6/16 clk: starfive: Add JH7100 clock generator driver
In reply to Emil Renner BerthingFrom: Geert Uytterhoeven <[email protected]>
Add a driver for the StarFive JH7100 clock generator.
--- MAINTAINERS | 7 + drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 1 + drivers/clk/starfive/Kconfig | 9 + drivers/clk/starfive/Makefile | 3 + drivers/clk/starfive/clk-starfive-jh7100.c | 689 +++++++++++++++++++++ 6 files changed, 710 insertions(+) create mode 100644 drivers/clk/starfive/Kconfig create mode 100644 drivers/clk/starfive/Makefile create mode 100644 drivers/clk/starfive/clk-starfive-jh7100.c
MAINTAINERS +7 -0
@@ -17859,6 +17859,13 @@ M: Ion Badulescu <[email protected]> S: Odd Fixes F: drivers/net/ethernet/adaptec/starfire* +STARFIVE JH7100 CLOCK DRIVER +M: Emil Renner Berthing <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml +F: drivers/clk/starfive/clk-starfive-jh7100.c +F: include/dt-bindings/clock/starfive-jh7100.h + STATIC BRANCH/CALL M: Peter Zijlstra <[email protected]> M: Josh Poimboeuf <[email protected]>
drivers/clk/Kconfig +1 -0
@@ -412,6 +412,7 @@ source "drivers/clk/samsung/Kconfig" source "drivers/clk/sifive/Kconfig" source "drivers/clk/socfpga/Kconfig" source "drivers/clk/sprd/Kconfig" +source "drivers/clk/starfive/Kconfig" source "drivers/clk/sunxi/Kconfig" source "drivers/clk/sunxi-ng/Kconfig" source "drivers/clk/tegra/Kconfig"
drivers/clk/Makefile +1 -0
@@ -109,6 +109,7 @@ obj-y += socfpga/ obj-$(CONFIG_PLAT_SPEAR) += spear/ obj-y += sprd/ obj-$(CONFIG_ARCH_STI) += st/ +obj-$(CONFIG_SOC_STARFIVE) += starfive/ obj-$(CONFIG_ARCH_SUNXI) += sunxi/ obj-$(CONFIG_SUNXI_CCU) += sunxi-ng/ obj-$(CONFIG_ARCH_TEGRA) += tegra/
drivers/clk/starfive/Kconfig new file +9 -0
@@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0 + +config CLK_STARFIVE_JH7100 + bool "StarFive JH7100 clock support" + depends on SOC_STARFIVE || COMPILE_TEST + default SOC_STARFIVE + help + Say yes here to support the clock controller on the StarFive JH7100 + SoC.
drivers/clk/starfive/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 +# StarFive Clock +obj-$(CONFIG_CLK_STARFIVE_JH7100) += clk-starfive-jh7100.o
drivers/clk/starfive/clk-starfive-jh7100.c new file +689 -0
689 lines not on this page · raw message
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 06/16] clk: starfive: Add JH7100 clock generator driver
Quoted 4 lines · Emil Renner Berthing
From: Geert Uytterhoeven <[email protected]>
Add a driver for the StarFive JH7100 clock generator.
FWIW,
Quoted 779 lines
Signed-off-by Geert Uytterhoeven <[email protected]>Co-developed-by Emil Renner Berthing <[email protected]>Signed-off-by Emil Renner Berthing <[email protected]>--- MAINTAINERS | 7 + drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 1 + drivers/clk/starfive/Kconfig | 9 + drivers/clk/starfive/Makefile | 3 + drivers/clk/starfive/clk-starfive-jh7100.c | 689 +++++++++++++++++++++ 6 files changed, 710 insertions(+) create mode 100644 drivers/clk/starfive/Kconfig create mode 100644 drivers/clk/starfive/Makefile create mode 100644 drivers/clk/starfive/clk-starfive-jh7100.cMAINTAINERS +7
-0@@ -17859,6 +17859,13 @@ M: Ion Badulescu <[email protected]> S: Odd Fixes F: drivers/net/ethernet/adaptec/starfire* +STARFIVE JH7100 CLOCK DRIVER +M: Emil Renner Berthing <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml +F: drivers/clk/starfive/clk-starfive-jh7100.c +F: include/dt-bindings/clock/starfive-jh7100.h + STATIC BRANCH/CALL M: Peter Zijlstra <[email protected]> M: Josh Poimboeuf <[email protected]>drivers/clk/Kconfig +1
-0@@ -412,6 +412,7 @@ source "drivers/clk/samsung/Kconfig" source "drivers/clk/sifive/Kconfig" source "drivers/clk/socfpga/Kconfig" source "drivers/clk/sprd/Kconfig" +source "drivers/clk/starfive/Kconfig" source "drivers/clk/sunxi/Kconfig" source "drivers/clk/sunxi-ng/Kconfig" source "drivers/clk/tegra/Kconfig"drivers/clk/Makefile +1
-0@@ -109,6 +109,7 @@ obj-y += socfpga/ obj-$(CONFIG_PLAT_SPEAR) += spear/ obj-y += sprd/ obj-$(CONFIG_ARCH_STI) += st/ +obj-$(CONFIG_SOC_STARFIVE) += starfive/ obj-$(CONFIG_ARCH_SUNXI) += sunxi/ obj-$(CONFIG_SUNXI_CCU) += sunxi-ng/ obj-$(CONFIG_ARCH_TEGRA) += tegra/drivers/clk/starfive/Kconfig new file +9
-0@@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0 + +config CLK_STARFIVE_JH7100 + bool "StarFive JH7100 clock support" + depends on SOC_STARFIVE || COMPILE_TEST + default SOC_STARFIVE + help + Say yes here to support the clock controller on the StarFive JH7100 + SoC.drivers/clk/starfive/Makefile new file +3
-0@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 +# StarFive Clock +obj-$(CONFIG_CLK_STARFIVE_JH7100) += clk-starfive-jh7100.odrivers/clk/starfive/clk-starfive-jh7100.c new file +689
-1@@ -0,0 +1,689 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* StarFive JH7100 Clock Generator Driver +* +* Copyright 2021 Ahmad Fatoum, Pengutronix +* Copyright (C) 2021 Glider bv +* Copyright (C) 2021 Emil Renner Berthing <[email protected]> +*/ + +#include <linux/bits.h> +#include <linux/clk-provider.h> +#include <linux/debugfs.h> +#include <linux/device.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> + +#include <dt-bindings/clock/starfive-jh7100.h> + +/* external clocks */ +#define JH7100_CLK_OSC_SYS (JH7100_CLK_END + 0) +#define JH7100_CLK_OSC_AUD (JH7100_CLK_END + 1) +#define JH7100_CLK_GMAC_RMII_REF (JH7100_CLK_END + 2) +#define JH7100_CLK_GMAC_GR_MII_RX (JH7100_CLK_END + 3) + +/* register fields */ +#define JH7100_CLK_ENABLE BIT(31) +#define JH7100_CLK_INVERT BIT(30) +#define JH7100_CLK_MUX_MASK GENMASK(27, 24) +#define JH7100_CLK_MUX_SHIFT 24 +#define JH7100_CLK_DIV_MASK GENMASK(23, 0) + +/* clock data */ +#define JH7100_GATE (_idx, _name, _flags, _parent) [_idx] = { \ + .name = _name, \ + .flags = CLK_SET_RATE_PARENT | (_flags), \ + .max = JH7100_CLK_ENABLE, \ + .parents = { [0] = _parent }, \ +} + +#define JH7100__DIV (_idx, _name, _max, _parent) [_idx] = { \ + .name = _name, \ + .flags = 0, \ + .max = _max, \ + .parents = { [0] = _parent }, \ +} + +#define JH7100_GDIV (_idx, _name, _flags, _max, _parent) [_idx] = { \ + .name = _name, \ + .flags = _flags, \ + .max = JH7100_CLK_ENABLE | (_max), \ + .parents = { [0] = _parent }, \ +} + +#define JH7100__MUX (_idx, _name, _nparents, ...) [_idx] = { \ + .name = _name, \ + .flags = 0, \ + .max = ((_nparents) - 1) << JH7100_CLK_MUX_SHIFT, \ + .parents = { __VA_ARGS__ }, \ +} + +#define JH7100_GMUX (_idx, _name, _flags, _nparents, ...) [_idx] = { \ + .name = _name, \ + .flags = _flags, \ + .max = JH7100_CLK_ENABLE | \ + (((_nparents) - 1) << JH7100_CLK_MUX_SHIFT), \ + .parents = { __VA_ARGS__ }, \ +} + +#define JH7100__INV (_idx, _name, _parent) [_idx] = { \ + .name = _name, \ + .flags = CLK_SET_RATE_PARENT, \ + .max = JH7100_CLK_INVERT, \ + .parents = { [0] = _parent }, \ +} + +static const struct { +const char *name; +unsigned long flags; +u32 max; +u8 parents[4 ]; +} jh7100_clk_data[]__initconst = { +JH7100__MUX (JH7100_CLK_CPUNDBUS_ROOT ,"cpundbus_root" ,4 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL1_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_DLA_ROOT ,"dla_root" ,3 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL1_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_DSP_ROOT ,"dsp_root" ,4 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL1_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_GMACUSB_ROOT ,"gmacusb_root" ,3 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_PERH0_ROOT ,"perh0_root" ,2 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL0_OUT ), +JH7100__MUX (JH7100_CLK_PERH1_ROOT ,"perh1_root" ,2 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_VIN_ROOT ,"vin_root" ,3 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL1_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_VOUT_ROOT ,"vout_root" ,3 , +JH7100_CLK_OSC_AUD , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100_GDIV (JH7100_CLK_AUDIO_ROOT ,"audio_root" ,0 ,8 ,JH7100_CLK_PLL0_OUT ), +JH7100__MUX (JH7100_CLK_CDECHIFI4_ROOT ,"cdechifi4_root" ,3 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL1_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__MUX (JH7100_CLK_CDEC_ROOT ,"cdec_root" ,3 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL1_OUT ), +JH7100__MUX (JH7100_CLK_VOUTBUS_ROOT ,"voutbus_root" ,3 , +JH7100_CLK_OSC_AUD , +JH7100_CLK_PLL0_OUT , +JH7100_CLK_PLL2_OUT ), +JH7100__DIV (JH7100_CLK_CPUNBUS_ROOT_DIV ,"cpunbus_root_div" ,2 ,JH7100_CLK_CPUNDBUS_ROOT ), +JH7100__DIV (JH7100_CLK_DSP_ROOT_DIV ,"dsp_root_div" ,4 ,JH7100_CLK_DSP_ROOT ), +JH7100__DIV (JH7100_CLK_PERH0_SRC ,"perh0_src" ,4 ,JH7100_CLK_PERH0_ROOT ), +JH7100__DIV (JH7100_CLK_PERH1_SRC ,"perh1_src" ,4 ,JH7100_CLK_PERH1_ROOT ), +JH7100_GDIV (JH7100_CLK_PLL0_TESTOUT ,"pll0_testout" ,0 ,31 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_PLL1_TESTOUT ,"pll1_testout" ,0 ,31 ,JH7100_CLK_DLA_ROOT ), +JH7100_GDIV (JH7100_CLK_PLL2_TESTOUT ,"pll2_testout" ,0 ,31 ,JH7100_CLK_PERH1_SRC ), +JH7100__MUX (JH7100_CLK_PLL2_REF ,"pll2_refclk" ,2 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_OSC_AUD ), +JH7100__DIV (JH7100_CLK_CPU_CORE ,"cpu_core" ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100__DIV (JH7100_CLK_CPU_AXI ,"cpu_axi" ,8 ,JH7100_CLK_CPU_CORE ), +JH7100__DIV (JH7100_CLK_AHB_BUS ,"ahb_bus" ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100__DIV (JH7100_CLK_APB1_BUS ,"apb1_bus" ,8 ,JH7100_CLK_AHB_BUS ), +JH7100__DIV (JH7100_CLK_APB2_BUS ,"apb2_bus" ,8 ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_DOM3AHB_BUS ,"dom3ahb_bus" ,CLK_IS_CRITICAL ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_DOM7AHB_BUS ,"dom7ahb_bus" ,CLK_IS_CRITICAL ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_U74_CORE0 ,"u74_core0" ,CLK_IS_CRITICAL ,JH7100_CLK_CPU_CORE ), +JH7100_GDIV (JH7100_CLK_U74_CORE1 ,"u74_core1" ,CLK_IS_CRITICAL ,8 ,JH7100_CLK_CPU_CORE ), +JH7100_GATE (JH7100_CLK_U74_AXI ,"u74_axi" ,CLK_IS_CRITICAL ,JH7100_CLK_CPU_AXI ), +JH7100_GATE (JH7100_CLK_U74RTC_TOGGLE ,"u74rtc_toggle" ,CLK_IS_CRITICAL ,JH7100_CLK_OSC_SYS ), +JH7100_GATE (JH7100_CLK_SGDMA2P_AXI ,"sgdma2p_axi" ,0 ,JH7100_CLK_CPU_AXI ), +JH7100_GATE (JH7100_CLK_DMA2PNOC_AXI ,"dma2pnoc_axi" ,0 ,JH7100_CLK_CPU_AXI ), +JH7100_GATE (JH7100_CLK_SGDMA2P_AHB ,"sgdma2p_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100__DIV (JH7100_CLK_DLA_BUS ,"dla_bus" ,4 ,JH7100_CLK_DLA_ROOT ), +JH7100_GATE (JH7100_CLK_DLA_AXI ,"dla_axi" ,0 ,JH7100_CLK_DLA_BUS ), +JH7100_GATE (JH7100_CLK_DLANOC_AXI ,"dlanoc_axi" ,0 ,JH7100_CLK_DLA_BUS ), +JH7100_GATE (JH7100_CLK_DLA_APB ,"dla_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_VP6_CORE ,"vp6_core" ,0 ,4 ,JH7100_CLK_DSP_ROOT_DIV ), +JH7100__DIV (JH7100_CLK_VP6BUS_SRC ,"vp6bus_src" ,4 ,JH7100_CLK_DSP_ROOT ), +JH7100_GDIV (JH7100_CLK_VP6_AXI ,"vp6_axi" ,0 ,4 ,JH7100_CLK_VP6BUS_SRC ), +JH7100__DIV (JH7100_CLK_VCDECBUS_SRC ,"vcdecbus_src" ,4 ,JH7100_CLK_CDECHIFI4_ROOT ), +JH7100__DIV (JH7100_CLK_VDEC_BUS ,"vdec_bus" ,8 ,JH7100_CLK_VCDECBUS_SRC ), +JH7100_GATE (JH7100_CLK_VDEC_AXI ,"vdec_axi" ,0 ,JH7100_CLK_VDEC_BUS ), +JH7100_GATE (JH7100_CLK_VDECBRG_MAIN ,"vdecbrg_mainclk" ,0 ,JH7100_CLK_VDEC_BUS ), +JH7100_GDIV (JH7100_CLK_VDEC_BCLK ,"vdec_bclk" ,0 ,8 ,JH7100_CLK_VCDECBUS_SRC ), +JH7100_GDIV (JH7100_CLK_VDEC_CCLK ,"vdec_cclk" ,0 ,8 ,JH7100_CLK_CDEC_ROOT ), +JH7100_GATE (JH7100_CLK_VDEC_APB ,"vdec_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_JPEG_AXI ,"jpeg_axi" ,0 ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100_GDIV (JH7100_CLK_JPEG_CCLK ,"jpeg_cclk" ,0 ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100_GATE (JH7100_CLK_JPEG_APB ,"jpeg_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_GC300_2X ,"gc300_2x" ,0 ,8 ,JH7100_CLK_CDECHIFI4_ROOT ), +JH7100_GATE (JH7100_CLK_GC300_AHB ,"gc300_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100__DIV (JH7100_CLK_JPCGC300_AXIBUS ,"jpcgc300_axibus" ,8 ,JH7100_CLK_VCDECBUS_SRC ), +JH7100_GATE (JH7100_CLK_GC300_AXI ,"gc300_axi" ,0 ,JH7100_CLK_JPCGC300_AXIBUS ), +JH7100_GATE (JH7100_CLK_JPCGC300_MAIN ,"jpcgc300_mainclk" ,0 ,JH7100_CLK_JPCGC300_AXIBUS ), +JH7100__DIV (JH7100_CLK_VENC_BUS ,"venc_bus" ,8 ,JH7100_CLK_VCDECBUS_SRC ), +JH7100_GATE (JH7100_CLK_VENC_AXI ,"venc_axi" ,0 ,JH7100_CLK_VENC_BUS ), +JH7100_GATE (JH7100_CLK_VENCBRG_MAIN ,"vencbrg_mainclk" ,0 ,JH7100_CLK_VENC_BUS ), +JH7100_GDIV (JH7100_CLK_VENC_BCLK ,"venc_bclk" ,0 ,8 ,JH7100_CLK_VCDECBUS_SRC ), +JH7100_GDIV (JH7100_CLK_VENC_CCLK ,"venc_cclk" ,0 ,8 ,JH7100_CLK_CDEC_ROOT ), +JH7100_GATE (JH7100_CLK_VENC_APB ,"venc_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_DDRPLL_DIV2 ,"ddrpll_div2" ,CLK_IS_CRITICAL ,2 ,JH7100_CLK_PLL1_OUT ), +JH7100_GDIV (JH7100_CLK_DDRPLL_DIV4 ,"ddrpll_div4" ,CLK_IS_CRITICAL ,2 ,JH7100_CLK_DDRPLL_DIV2 ), +JH7100_GDIV (JH7100_CLK_DDRPLL_DIV8 ,"ddrpll_div8" ,CLK_IS_CRITICAL ,2 ,JH7100_CLK_DDRPLL_DIV4 ), +JH7100_GDIV (JH7100_CLK_DDROSC_DIV2 ,"ddrosc_div2" ,CLK_IS_CRITICAL ,2 ,JH7100_CLK_OSC_SYS ), +JH7100_GMUX (JH7100_CLK_DDRC0 ,"ddrc0" ,CLK_IS_CRITICAL ,4 , +JH7100_CLK_DDROSC_DIV2 , +JH7100_CLK_DDRPLL_DIV2 , +JH7100_CLK_DDRPLL_DIV4 , +JH7100_CLK_DDRPLL_DIV8 ), +JH7100_GMUX (JH7100_CLK_DDRC1 ,"ddrc1" ,CLK_IS_CRITICAL ,4 , +JH7100_CLK_DDROSC_DIV2 , +JH7100_CLK_DDRPLL_DIV2 , +JH7100_CLK_DDRPLL_DIV4 , +JH7100_CLK_DDRPLL_DIV8 ), +JH7100_GATE (JH7100_CLK_DDRPHY_APB ,"ddrphy_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100__DIV (JH7100_CLK_NOC_ROB ,"noc_rob" ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100__DIV (JH7100_CLK_NOC_COG ,"noc_cog" ,8 ,JH7100_CLK_DLA_ROOT ), +JH7100_GATE (JH7100_CLK_NNE_AHB ,"nne_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100__DIV (JH7100_CLK_NNEBUS_SRC1 ,"nnebus_src1" ,4 ,JH7100_CLK_DSP_ROOT ), +JH7100__MUX (JH7100_CLK_NNE_BUS ,"nne_bus" ,2 , +JH7100_CLK_CPU_AXI , +JH7100_CLK_NNEBUS_SRC1 ), +JH7100_GATE (JH7100_CLK_NNE_AXI ,"nne_axi" ,0 ,JH7100_CLK_NNE_BUS ), +JH7100_GATE (JH7100_CLK_NNENOC_AXI ,"nnenoc_axi" ,0 ,JH7100_CLK_NNE_BUS ), +JH7100_GATE (JH7100_CLK_DLASLV_AXI ,"dlaslv_axi" ,0 ,JH7100_CLK_NNE_BUS ), +JH7100_GATE (JH7100_CLK_DSPX2C_AXI ,"dspx2c_axi" ,CLK_IS_CRITICAL ,JH7100_CLK_NNE_BUS ), +JH7100__DIV (JH7100_CLK_HIFI4_SRC ,"hifi4_src" ,4 ,JH7100_CLK_CDECHIFI4_ROOT ), +JH7100__DIV (JH7100_CLK_HIFI4_COREFREE ,"hifi4_corefree" ,8 ,JH7100_CLK_HIFI4_SRC ), +JH7100_GATE (JH7100_CLK_HIFI4_CORE ,"hifi4_core" ,0 ,JH7100_CLK_HIFI4_COREFREE ), +JH7100__DIV (JH7100_CLK_HIFI4_BUS ,"hifi4_bus" ,8 ,JH7100_CLK_HIFI4_COREFREE ), +JH7100_GATE (JH7100_CLK_HIFI4_AXI ,"hifi4_axi" ,0 ,JH7100_CLK_HIFI4_BUS ), +JH7100_GATE (JH7100_CLK_HIFI4NOC_AXI ,"hifi4noc_axi" ,0 ,JH7100_CLK_HIFI4_BUS ), +JH7100__DIV (JH7100_CLK_SGDMA1P_BUS ,"sgdma1p_bus" ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100_GATE (JH7100_CLK_SGDMA1P_AXI ,"sgdma1p_axi" ,0 ,JH7100_CLK_SGDMA1P_BUS ), +JH7100_GATE (JH7100_CLK_DMA1P_AXI ,"dma1p_axi" ,0 ,JH7100_CLK_SGDMA1P_BUS ), +JH7100_GDIV (JH7100_CLK_X2C_AXI ,"x2c_axi" ,CLK_IS_CRITICAL ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100__DIV (JH7100_CLK_USB_BUS ,"usb_bus" ,8 ,JH7100_CLK_CPUNBUS_ROOT_DIV ), +JH7100_GATE (JH7100_CLK_USB_AXI ,"usb_axi" ,0 ,JH7100_CLK_USB_BUS ), +JH7100_GATE (JH7100_CLK_USBNOC_AXI ,"usbnoc_axi" ,0 ,JH7100_CLK_USB_BUS ), +JH7100__DIV (JH7100_CLK_USBPHY_ROOTDIV ,"usbphy_rootdiv" ,4 ,JH7100_CLK_GMACUSB_ROOT ), +JH7100_GDIV (JH7100_CLK_USBPHY_125M ,"usbphy_125m" ,0 ,8 ,JH7100_CLK_USBPHY_ROOTDIV ), +JH7100_GDIV (JH7100_CLK_USBPHY_PLLDIV25M ,"usbphy_plldiv25m" ,0 ,32 ,JH7100_CLK_USBPHY_ROOTDIV ), +JH7100__MUX (JH7100_CLK_USBPHY_25M ,"usbphy_25m" ,2 , +JH7100_CLK_OSC_SYS , +JH7100_CLK_USBPHY_PLLDIV25M ), +JH7100__DIV (JH7100_CLK_AUDIO_DIV ,"audio_div" ,131072 ,JH7100_CLK_AUDIO_ROOT ), +JH7100_GATE (JH7100_CLK_AUDIO_SRC ,"audio_src" ,0 ,JH7100_CLK_AUDIO_DIV ), +JH7100_GATE (JH7100_CLK_AUDIO_12288 ,"audio_12288" ,0 ,JH7100_CLK_OSC_AUD ), +JH7100_GDIV (JH7100_CLK_VIN_SRC ,"vin_src" ,0 ,4 ,JH7100_CLK_VIN_ROOT ), +JH7100__DIV (JH7100_CLK_ISP0_BUS ,"isp0_bus" ,8 ,JH7100_CLK_VIN_SRC ), +JH7100_GATE (JH7100_CLK_ISP0_AXI ,"isp0_axi" ,0 ,JH7100_CLK_ISP0_BUS ), +JH7100_GATE (JH7100_CLK_ISP0NOC_AXI ,"isp0noc_axi" ,0 ,JH7100_CLK_ISP0_BUS ), +JH7100_GATE (JH7100_CLK_ISPSLV_AXI ,"ispslv_axi" ,0 ,JH7100_CLK_ISP0_BUS ), +JH7100__DIV (JH7100_CLK_ISP1_BUS ,"isp1_bus" ,8 ,JH7100_CLK_VIN_SRC ), +JH7100_GATE (JH7100_CLK_ISP1_AXI ,"isp1_axi" ,0 ,JH7100_CLK_ISP1_BUS ), +JH7100_GATE (JH7100_CLK_ISP1NOC_AXI ,"isp1noc_axi" ,0 ,JH7100_CLK_ISP1_BUS ), +JH7100__DIV (JH7100_CLK_VIN_BUS ,"vin_bus" ,8 ,JH7100_CLK_VIN_SRC ), +JH7100_GATE (JH7100_CLK_VIN_AXI ,"vin_axi" ,0 ,JH7100_CLK_VIN_BUS ), +JH7100_GATE (JH7100_CLK_VINNOC_AXI ,"vinnoc_axi" ,0 ,JH7100_CLK_VIN_BUS ), +JH7100_GDIV (JH7100_CLK_VOUT_SRC ,"vout_src" ,0 ,4 ,JH7100_CLK_VOUT_ROOT ), +JH7100__DIV (JH7100_CLK_DISPBUS_SRC ,"dispbus_src" ,4 ,JH7100_CLK_VOUTBUS_ROOT ), +JH7100__DIV (JH7100_CLK_DISP_BUS ,"disp_bus" ,4 ,JH7100_CLK_DISPBUS_SRC ), +JH7100_GATE (JH7100_CLK_DISP_AXI ,"disp_axi" ,0 ,JH7100_CLK_DISP_BUS ), +JH7100_GATE (JH7100_CLK_DISPNOC_AXI ,"dispnoc_axi" ,0 ,JH7100_CLK_DISP_BUS ), +JH7100_GATE (JH7100_CLK_SDIO0_AHB ,"sdio0_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GDIV (JH7100_CLK_SDIO0_CCLKINT ,"sdio0_cclkint" ,0 ,24 ,JH7100_CLK_PERH0_SRC ), +JH7100__INV (JH7100_CLK_SDIO0_CCLKINT_INV ,"sdio0_cclkint_inv" ,JH7100_CLK_SDIO0_CCLKINT ), +JH7100_GATE (JH7100_CLK_SDIO1_AHB ,"sdio1_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GDIV (JH7100_CLK_SDIO1_CCLKINT ,"sdio1_cclkint" ,0 ,24 ,JH7100_CLK_PERH1_SRC ), +JH7100__INV (JH7100_CLK_SDIO1_CCLKINT_INV ,"sdio1_cclkint_inv" ,JH7100_CLK_SDIO1_CCLKINT ), +JH7100_GATE (JH7100_CLK_GMAC_AHB ,"gmac_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100__DIV (JH7100_CLK_GMAC_ROOT_DIV ,"gmac_root_div" ,8 ,JH7100_CLK_GMACUSB_ROOT ), +JH7100_GDIV (JH7100_CLK_GMAC_PTP_REF ,"gmac_ptp_refclk" ,0 ,31 ,JH7100_CLK_GMAC_ROOT_DIV ), +JH7100_GDIV (JH7100_CLK_GMAC_GTX ,"gmac_gtxclk" ,0 ,255 ,JH7100_CLK_GMAC_ROOT_DIV ), +JH7100_GDIV (JH7100_CLK_GMAC_RMII_TX ,"gmac_rmii_txclk" ,0 ,8 ,JH7100_CLK_GMAC_RMII_REF ), +JH7100_GDIV (JH7100_CLK_GMAC_RMII_RX ,"gmac_rmii_rxclk" ,0 ,8 ,JH7100_CLK_GMAC_RMII_REF ), +JH7100__MUX (JH7100_CLK_GMAC_TX ,"gmac_tx" ,3 , +JH7100_CLK_GMAC_GTX , +JH7100_CLK_GMAC_TX_INV , +JH7100_CLK_GMAC_RMII_TX ), +JH7100__INV (JH7100_CLK_GMAC_TX_INV ,"gmac_tx_inv" ,JH7100_CLK_GMAC_TX ), +JH7100__MUX (JH7100_CLK_GMAC_RX_PRE ,"gmac_rx_pre" ,2 , +JH7100_CLK_GMAC_GR_MII_RX , +JH7100_CLK_GMAC_RMII_RX ), +JH7100__INV (JH7100_CLK_GMAC_RX_INV ,"gmac_rx_inv" ,JH7100_CLK_GMAC_RX_PRE ), +JH7100_GATE (JH7100_CLK_GMAC_RMII ,"gmac_rmii" ,0 ,JH7100_CLK_GMAC_RMII_REF ), +JH7100_GDIV (JH7100_CLK_GMAC_TOPHYREF ,"gmac_tophyref" ,0 ,127 ,JH7100_CLK_GMAC_ROOT_DIV ), +JH7100_GATE (JH7100_CLK_SPI2AHB_AHB ,"spi2ahb_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GDIV (JH7100_CLK_SPI2AHB_CORE ,"spi2ahb_core" ,0 ,31 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_EZMASTER_AHB ,"ezmaster_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_E24_AHB ,"e24_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_E24RTC_TOGGLE ,"e24rtc_toggle" ,0 ,JH7100_CLK_OSC_SYS ), +JH7100_GATE (JH7100_CLK_QSPI_AHB ,"qspi_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_QSPI_APB ,"qspi_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_QSPI_REF ,"qspi_refclk" ,0 ,31 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_SEC_AHB ,"sec_ahb" ,0 ,JH7100_CLK_AHB_BUS ), +JH7100_GATE (JH7100_CLK_AES ,"aes_clk" ,0 ,JH7100_CLK_SEC_AHB ), +JH7100_GATE (JH7100_CLK_SHA ,"sha_clk" ,0 ,JH7100_CLK_SEC_AHB ), +JH7100_GATE (JH7100_CLK_PKA ,"pka_clk" ,0 ,JH7100_CLK_SEC_AHB ), +JH7100_GATE (JH7100_CLK_TRNG_APB ,"trng_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GATE (JH7100_CLK_OTP_APB ,"otp_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GATE (JH7100_CLK_UART0_APB ,"uart0_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_UART0_CORE ,"uart0_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_UART1_APB ,"uart1_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_UART1_CORE ,"uart1_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_SPI0_APB ,"spi0_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_SPI0_CORE ,"spi0_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_SPI1_APB ,"spi1_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_SPI1_CORE ,"spi1_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_I2C0_APB ,"i2c0_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_I2C0_CORE ,"i2c0_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_I2C1_APB ,"i2c1_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GDIV (JH7100_CLK_I2C1_CORE ,"i2c1_core" ,0 ,63 ,JH7100_CLK_PERH1_SRC ), +JH7100_GATE (JH7100_CLK_GPIO_APB ,"gpio_apb" ,0 ,JH7100_CLK_APB1_BUS ), +JH7100_GATE (JH7100_CLK_UART2_APB ,"uart2_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_UART2_CORE ,"uart2_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_UART3_APB ,"uart3_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_UART3_CORE ,"uart3_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_SPI2_APB ,"spi2_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_SPI2_CORE ,"spi2_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_SPI3_APB ,"spi3_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_SPI3_CORE ,"spi3_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_I2C2_APB ,"i2c2_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_I2C2_CORE ,"i2c2_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_I2C3_APB ,"i2c3_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_I2C3_CORE ,"i2c3_core" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_WDTIMER_APB ,"wdtimer_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_WDT_CORE ,"wdt_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER0_CORE ,"timer0_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER1_CORE ,"timer1_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER2_CORE ,"timer2_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER3_CORE ,"timer3_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER4_CORE ,"timer4_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER5_CORE ,"timer5_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GDIV (JH7100_CLK_TIMER6_CORE ,"timer6_coreclk" ,0 ,63 ,JH7100_CLK_PERH0_SRC ), +JH7100_GATE (JH7100_CLK_VP6INTC_APB ,"vp6intc_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GATE (JH7100_CLK_PWM_APB ,"pwm_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GATE (JH7100_CLK_MSI_APB ,"msi_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GATE (JH7100_CLK_TEMP_APB ,"temp_apb" ,0 ,JH7100_CLK_APB2_BUS ), +JH7100_GDIV (JH7100_CLK_TEMP_SENSE ,"temp_sense" ,0 ,31 ,JH7100_CLK_OSC_SYS ), +JH7100_GATE (JH7100_CLK_SYSERR_APB ,"syserr_apb" ,0 ,JH7100_CLK_APB2_BUS ), +}; + +struct jh7100_clk { +struct clk_hw hw; +unsigned int idx; +unsigned int max_div; +}; + +struct clk_starfive_jh7100_priv { +/* protect clk enable and set rate/parent from happening at the same time */ +spinlock_t rmw_lock; +struct device *dev; +void __iomem *base; +struct clk_hw *pll[3 ]; +struct jh7100_clk reg[JH7100_CLK_PLL0_OUT ]; +}; + +static struct jh7100_clk *jh7100_clk_from (struct clk_hw *hw) +{ + return container_of(hw, struct jh7100_clk, hw); +} + +static struct clk_starfive_jh7100_priv *jh7100_priv_from (struct jh7100_clk *clk) +{ + return container_of(clk, struct clk_starfive_jh7100_priv, reg[clk->idx]); +} + +static u32 jh7100_clk_reg_get (struct jh7100_clk *clk) +{ +struct clk_starfive_jh7100_priv *priv =jh7100_priv_from (clk); +void __iomem *reg = priv->base +4 * clk->idx; + +return readl_relaxed (reg); +} + +static void jh7100_clk_reg_rmw (struct jh7100_clk *clk,u32 mask,u32 value) +{ +struct clk_starfive_jh7100_priv *priv =jh7100_priv_from (clk); +void __iomem *reg = priv->base +4 * clk->idx; +unsigned long flags; + +spin_lock_irqsave (&priv->rmw_lock, flags); + value |=readl_relaxed (reg) & ~mask; +writel_relaxed (value, reg); +spin_unlock_irqrestore (&priv->rmw_lock, flags); +} + +static int jh7100_clk_enable (struct clk_hw *hw) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); + +jh7100_clk_reg_rmw (clk,JH7100_CLK_ENABLE ,JH7100_CLK_ENABLE ); +return 0 ; +} + +static void jh7100_clk_disable (struct clk_hw *hw) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); + +jh7100_clk_reg_rmw (clk,JH7100_CLK_ENABLE ,0 ); +} + +static int jh7100_clk_is_enabled (struct clk_hw *hw) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); + +return !!(jh7100_clk_reg_get (clk) &JH7100_CLK_ENABLE ); +} + +static unsigned long jh7100_clk_recalc_rate (struct clk_hw *hw, +unsigned long parent_rate) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +u32 div =jh7100_clk_reg_get (clk) &JH7100_CLK_DIV_MASK ; + +return div ? parent_rate / div :0 ; +} + +static unsigned long jh7100_clk_bestdiv (struct jh7100_clk *clk, +unsigned long rate,unsigned long parent) +{ +unsigned long max = clk->max_div; +unsigned long div =DIV_ROUND_UP (parent, rate); + +return min (div, max); +} + +static int jh7100_clk_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +unsigned long parent = req->best_parent_rate; +unsigned long rate =clamp (req->rate, req->min_rate, req->max_rate); +unsigned long div =jh7100_clk_bestdiv (clk, rate, parent); +unsigned long result = parent / div; + +/* +* we want the result clamped by min_rate and max_rate if possible: +* case 1: div hits the max divider value, which means it's less than +* parent / rate, so the result is greater than rate and min_rate in +* particular. we can't do anything about result > max_rate because the +* divider doesn't go any further. +* case 2: div = DIV_ROUND_UP(parent, rate) which means the result is +* always lower or equal to rate and max_rate. however the result may +* turn out lower than min_rate, but then the next higher rate is fine: +* div - 1 = ceil(parent / rate) - 1 < parent / rate +* and thus +* min_rate <= rate < parent / (div - 1) +*/ +if (result < req->min_rate && div >1 ) + result = parent / (div -1 ); + + req->rate = result; +return 0 ; +} + +static int jh7100_clk_set_rate (struct clk_hw *hw, +unsigned long rate, +unsigned long parent_rate) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +unsigned long div =jh7100_clk_bestdiv (clk, rate, parent_rate); + +jh7100_clk_reg_rmw (clk,JH7100_CLK_DIV_MASK , div); +return 0 ; +} + +static u8 jh7100_clk_get_parent (struct clk_hw *hw) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +u32 value =jh7100_clk_reg_get (clk); + +return (value &JH7100_CLK_MUX_MASK ) >>JH7100_CLK_MUX_SHIFT ; +} + +static int jh7100_clk_set_parent (struct clk_hw *hw,u8 index) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +u32 value = (u32 )index <<JH7100_CLK_MUX_SHIFT ; + +jh7100_clk_reg_rmw (clk,JH7100_CLK_MUX_MASK , value); +return 0 ; +} + +static int jh7100_clk_mux_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ +return clk_mux_determine_rate_flags (hw, req,0 ); +} + +static int jh7100_clk_get_phase (struct clk_hw *hw) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +u32 value =jh7100_clk_reg_get (clk); + +return (value &JH7100_CLK_INVERT ) ?180 :0 ; +} + +static int jh7100_clk_set_phase (struct clk_hw *hw,int degrees) +{ +struct jh7100_clk *clk =jh7100_clk_from (hw); +u32 value; + +if (degrees ==0 ) + value =0 ; +else if (degrees ==180 ) + value =JH7100_CLK_INVERT ; +else +return -EINVAL ; + +jh7100_clk_reg_rmw (clk,JH7100_CLK_INVERT , value); +return 0 ; +} + +#ifdef CONFIG_DEBUG_FS +static void jh7100_clk_debug_init (struct clk_hw *hw,struct dentry *dentry) +{ +static const struct debugfs_reg32 jh7100_clk_reg = { + .name ="CTRL" , + .offset =0 , + }; +struct jh7100_clk *clk =jh7100_clk_from (hw); +struct clk_starfive_jh7100_priv *priv =jh7100_priv_from (clk); +struct debugfs_regset32 *regset; + + regset =devm_kzalloc (priv->dev,sizeof (*regset),GFP_KERNEL ); +if (!regset) +return ; + + regset->regs = &jh7100_clk_reg; + regset->nregs =1 ; + regset->base = priv->base +4 * clk->idx; + +debugfs_create_regset32 ("registers" ,0400 , dentry, regset); +} +#else +#define jh7100_clk_debug_init NULL +#endif + +static const struct clk_ops jh7100_clk_gate_ops = { + .enable = jh7100_clk_enable, + .disable = jh7100_clk_disable, + .is_enabled = jh7100_clk_is_enabled, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops jh7100_clk_div_ops = { + .recalc_rate = jh7100_clk_recalc_rate, + .determine_rate = jh7100_clk_determine_rate, + .set_rate = jh7100_clk_set_rate, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops jh7100_clk_gdiv_ops = { + .enable = jh7100_clk_enable, + .disable = jh7100_clk_disable, + .is_enabled = jh7100_clk_is_enabled, + .recalc_rate = jh7100_clk_recalc_rate, + .determine_rate = jh7100_clk_determine_rate, + .set_rate = jh7100_clk_set_rate, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops jh7100_clk_mux_ops = { + .get_parent = jh7100_clk_get_parent, + .set_parent = jh7100_clk_set_parent, + .determine_rate = jh7100_clk_mux_determine_rate, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops jh7100_clk_gmux_ops = { + .enable = jh7100_clk_enable, + .disable = jh7100_clk_disable, + .is_enabled = jh7100_clk_is_enabled, + .get_parent = jh7100_clk_get_parent, + .set_parent = jh7100_clk_set_parent, + .determine_rate = jh7100_clk_mux_determine_rate, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops jh7100_clk_inv_ops = { + .get_phase = jh7100_clk_get_phase, + .set_phase = jh7100_clk_set_phase, + .debug_init = jh7100_clk_debug_init, +}; + +static const struct clk_ops *__init jh7100_clk_ops (u32 max) +{ +if (max &JH7100_CLK_DIV_MASK ) { +if (max &JH7100_CLK_ENABLE ) +return &jh7100_clk_gdiv_ops; +return &jh7100_clk_div_ops; + } + +if (max &JH7100_CLK_MUX_MASK ) { +if (max &JH7100_CLK_ENABLE ) +return &jh7100_clk_gmux_ops; +return &jh7100_clk_mux_ops; + } + +if (max &JH7100_CLK_ENABLE ) +return &jh7100_clk_gate_ops; + +return &jh7100_clk_inv_ops; +} + +static struct clk_hw *clk_starfive_jh7100_get (struct of_phandle_args *clkspec,void *data) +{ +struct clk_starfive_jh7100_priv *priv = data; +unsigned int idx = clkspec->args[0 ]; + +if (idx <JH7100_CLK_PLL0_OUT ) +return &priv->reg[idx].hw; + +if (idx <JH7100_CLK_END ) +return priv->pll[idx -JH7100_CLK_PLL0_OUT ]; + +return ERR_PTR (-EINVAL ); +} + +static int __init clk_starfive_jh7100_probe (struct platform_device *pdev) +{ +struct clk_starfive_jh7100_priv *priv; +unsigned int idx; +int ret; + + priv =devm_kzalloc (&pdev->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + +spin_lock_init (&priv->rmw_lock); + priv->dev = &pdev->dev; + priv->base =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (priv->base)) +return PTR_ERR (priv->base); + + priv->pll[0 ] =devm_clk_hw_register_fixed_factor (priv->dev,"pll0_out" , +"osc_sys" ,0 ,40 ,1 ); +if (IS_ERR (priv->pll[0 ])) +return PTR_ERR (priv->pll[0 ]); + + priv->pll[1 ] =devm_clk_hw_register_fixed_factor (priv->dev,"pll1_out" , +"osc_sys" ,0 ,64 ,1 ); +if (IS_ERR (priv->pll[1 ])) +return PTR_ERR (priv->pll[1 ]); + + priv->pll[2 ] =devm_clk_hw_register_fixed_factor (priv->dev,"pll2_out" , +"pll2_refclk" ,0 ,55 ,1 ); +if (IS_ERR (priv->pll[2 ])) +return PTR_ERR (priv->pll[2 ]); + +for (idx =0 ; idx <JH7100_CLK_PLL0_OUT ; idx++) { +u32 max = jh7100_clk_data[idx].max; +struct clk_parent_data parents[4 ] = {}; +struct clk_init_data init = { + .name = jh7100_clk_data[idx].name, + .ops =jh7100_clk_ops (max), + .parent_data = parents, + .num_parents = ((max &JH7100_CLK_MUX_MASK ) >>JH7100_CLK_MUX_SHIFT ) +1 , + .flags = jh7100_clk_data[idx].flags, + }; +struct jh7100_clk *clk = &priv->reg[idx]; +unsigned int i; + +for (i =0 ; i < init.num_parents; i++) { +unsigned int pidx = jh7100_clk_data[idx].parents[i]; + +if (pidx <JH7100_CLK_PLL0_OUT ) + parents[i].hw = &priv->reg[pidx].hw; +else if (pidx <JH7100_CLK_END ) + parents[i].hw = priv->pll[pidx -JH7100_CLK_PLL0_OUT ]; +else if (pidx ==JH7100_CLK_OSC_SYS ) + parents[i].fw_name ="osc_sys" ; +else if (pidx ==JH7100_CLK_OSC_AUD ) + parents[i].fw_name ="osc_aud" ; +else if (pidx ==JH7100_CLK_GMAC_RMII_REF ) + parents[i].fw_name ="gmac_rmii_ref" ; +else if (pidx ==JH7100_CLK_GMAC_GR_MII_RX ) + parents[i].fw_name ="gmac_gr_mii_rxclk" ; + } + + clk->hw.init = &init; + clk->idx = idx; + clk->max_div = max &JH7100_CLK_DIV_MASK ; + + ret =devm_clk_hw_register (priv->dev, &clk->hw); +if (ret) +return ret; + } + +return devm_of_clk_add_hw_provider (priv->dev, clk_starfive_jh7100_get, priv); +} + +static const struct of_device_id clk_starfive_jh7100_match[] = { + { .compatible ="starfive,jh7100-clkgen" }, + {/* sentinel */ } +}; + +static struct platform_driver clk_starfive_jh7100_driver = { + .driver = { + .name ="clk-starfive-jh7100" , + .of_match_table = clk_starfive_jh7100_match, + .suppress_bind_attrs = true, + }, +}; +builtin_platform_driver_probe (clk_starfive_jh7100_driver, clk_starfive_jh7100_probe); --2.33.1
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerPATCH v3 7/16 dt-bindings: reset: Add StarFive JH7100 reset definitions
In reply to Emil Renner BerthingFrom: Geert Uytterhoeven <[email protected]>
Add all resets for the StarFive JH7100 reset controller.
Based on work by Ahmad Fatoum for Barebox, with "JH7100_" prefixes added to all definitions.
--- include/dt-bindings/reset/starfive-jh7100.h | 126 ++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 include/dt-bindings/reset/starfive-jh7100.h
include/dt-bindings/reset/starfive-jh7100.h new file +126 -0
@@ -0,0 +1,126 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ +/* +* Copyright (C) 2021 Ahmad Fatoum, Pengutronix +*/ + +#ifndef __DT_BINDINGS_RESET_STARFIVE_JH7100_H__ +#define __DT_BINDINGS_RESET_STARFIVE_JH7100_H__ + +#define JH7100_RSTN_DOM3AHB_BUS 0 +#define JH7100_RSTN_DOM7AHB_BUS 1 +#define JH7100_RST_U74 2 +#define JH7100_RSTN_U74_AXI 3 +#define JH7100_RSTN_SGDMA2P_AHB 4 +#define JH7100_RSTN_SGDMA2P_AXI 5 +#define JH7100_RSTN_DMA2PNOC_AXI 6 +#define JH7100_RSTN_DLA_AXI 7 +#define JH7100_RSTN_DLANOC_AXI 8 +#define JH7100_RSTN_DLA_APB 9 +#define JH7100_RST_VP6_DRESET 10 +#define JH7100_RST_VP6_BRESET 11 +#define JH7100_RSTN_VP6_AXI 12 +#define JH7100_RSTN_VDECBRG_MAIN 13 +#define JH7100_RSTN_VDEC_AXI 14 +#define JH7100_RSTN_VDEC_BCLK 15 +#define JH7100_RSTN_VDEC_CCLK 16 +#define JH7100_RSTN_VDEC_APB 17 +#define JH7100_RSTN_JPEG_AXI 18 +#define JH7100_RSTN_JPEG_CCLK 19 +#define JH7100_RSTN_JPEG_APB 20 +#define JH7100_RSTN_JPCGC300_MAIN 21 +#define JH7100_RSTN_GC300_2X 22 +#define JH7100_RSTN_GC300_AXI 23 +#define JH7100_RSTN_GC300_AHB 24 +#define JH7100_RSTN_VENC_AXI 25 +#define JH7100_RSTN_VENCBRG_MAIN 26 +#define JH7100_RSTN_VENC_BCLK 27 +#define JH7100_RSTN_VENC_CCLK 28 +#define JH7100_RSTN_VENC_APB 29 +#define JH7100_RSTN_DDRPHY_APB 30 +#define JH7100_RSTN_NOC_ROB 31 +#define JH7100_RSTN_NOC_COG 32 +#define JH7100_RSTN_HIFI4_AXI 33 +#define JH7100_RSTN_HIFI4NOC_AXI 34 +#define JH7100_RST_HIFI4_DRESET 35 +#define JH7100_RST_HIFI4_BRESET 36 +#define JH7100_RSTN_USB_AXI 37 +#define JH7100_RSTN_USBNOC_AXI 38 +#define JH7100_RSTN_SGDMA1P_AXI 39 +#define JH7100_RSTN_DMA1P_AXI 40 +#define JH7100_RSTN_X2C_AXI 41 +#define JH7100_RSTN_NNE_AHB 42 +#define JH7100_RSTN_NNE_AXI 43 +#define JH7100_RSTN_NNENOC_AXI 44 +#define JH7100_RSTN_DLASLV_AXI 45 +#define JH7100_RSTN_DSPX2C_AXI 46 +#define JH7100_RSTN_VIN_SRC 47 +#define JH7100_RSTN_ISPSLV_AXI 48 +#define JH7100_RSTN_VIN_AXI 49 +#define JH7100_RSTN_VINNOC_AXI 50 +#define JH7100_RSTN_ISP0_AXI 51 +#define JH7100_RSTN_ISP0NOC_AXI 52 +#define JH7100_RSTN_ISP1_AXI 53 +#define JH7100_RSTN_ISP1NOC_AXI 54 +#define JH7100_RSTN_VOUT_SRC 55 +#define JH7100_RSTN_DISP_AXI 56 +#define JH7100_RSTN_DISPNOC_AXI 57 +#define JH7100_RSTN_SDIO0_AHB 58 +#define JH7100_RSTN_SDIO1_AHB 59 +#define JH7100_RSTN_GMAC_AHB 60 +#define JH7100_RSTN_SPI2AHB_AHB 61 +#define JH7100_RSTN_SPI2AHB_CORE 62 +#define JH7100_RSTN_EZMASTER_AHB 63 +#define JH7100_RST_E24 64 +#define JH7100_RSTN_QSPI_AHB 65 +#define JH7100_RSTN_QSPI_CORE 66 +#define JH7100_RSTN_QSPI_APB 67 +#define JH7100_RSTN_SEC_AHB 68 +#define JH7100_RSTN_AES 69 +#define JH7100_RSTN_PKA 70 +#define JH7100_RSTN_SHA 71 +#define JH7100_RSTN_TRNG_APB 72 +#define JH7100_RSTN_OTP_APB 73 +#define JH7100_RSTN_UART0_APB 74 +#define JH7100_RSTN_UART0_CORE 75 +#define JH7100_RSTN_UART1_APB 76 +#define JH7100_RSTN_UART1_CORE 77 +#define JH7100_RSTN_SPI0_APB 78 +#define JH7100_RSTN_SPI0_CORE 79 +#define JH7100_RSTN_SPI1_APB 80 +#define JH7100_RSTN_SPI1_CORE 81 +#define JH7100_RSTN_I2C0_APB 82 +#define JH7100_RSTN_I2C0_CORE 83 +#define JH7100_RSTN_I2C1_APB 84 +#define JH7100_RSTN_I2C1_CORE 85 +#define JH7100_RSTN_GPIO_APB 86 +#define JH7100_RSTN_UART2_APB 87 +#define JH7100_RSTN_UART2_CORE 88 +#define JH7100_RSTN_UART3_APB 89 +#define JH7100_RSTN_UART3_CORE 90 +#define JH7100_RSTN_SPI2_APB 91 +#define JH7100_RSTN_SPI2_CORE 92 +#define JH7100_RSTN_SPI3_APB 93 +#define JH7100_RSTN_SPI3_CORE 94 +#define JH7100_RSTN_I2C2_APB 95 +#define JH7100_RSTN_I2C2_CORE 96 +#define JH7100_RSTN_I2C3_APB 97 +#define JH7100_RSTN_I2C3_CORE 98 +#define JH7100_RSTN_WDTIMER_APB 99 +#define JH7100_RSTN_WDT 100 +#define JH7100_RSTN_TIMER0 101 +#define JH7100_RSTN_TIMER1 102 +#define JH7100_RSTN_TIMER2 103 +#define JH7100_RSTN_TIMER3 104 +#define JH7100_RSTN_TIMER4 105 +#define JH7100_RSTN_TIMER5 106 +#define JH7100_RSTN_TIMER6 107 +#define JH7100_RSTN_VP6INTC_APB 108 +#define JH7100_RSTN_PWM_APB 109 +#define JH7100_RSTN_MSI_APB 110 +#define JH7100_RSTN_TEMP_APB 111 +#define JH7100_RSTN_TEMP_SENSE 112 +#define JH7100_RSTN_SYSERR_APB 113 + +#define JH7100_RSTN_END 114 + +#endif /* __DT_BINDINGS_RESET_STARFIVE_JH7100_H__ */
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 8/16 dt-bindings: reset: Add Starfive JH7100 reset bindings
In reply to Emil Renner BerthingAdd bindings for the reset controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
--- .../bindings/reset/starfive,jh7100-reset.yaml | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml new file +38 -0
@@ -0,0 +1,38 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/starfive,jh7100-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7100 SoC Reset Controller Device Tree Bindings + +maintainers: + - Emil Renner Berthing <[email protected]> + +properties: + compatible: + enum: + - starfive,jh7100-reset + + reg: + maxItems: 1 + + "#reset-cells": + const: 1 + +required: + - compatible + - reg + - "#reset-cells" + +additionalProperties: false + +examples: + - | + reset-controller@11840000 { + compatible = "starfive,jh7100-reset"; + reg = <0x11840000 0x10000>; + #reset-cells = <1>; + }; + +...
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 08/16] dt-bindings: reset: Add Starfive JH7100 reset bindings
Quoted 4 lines · Emil Renner Berthing
Add bindings for the reset controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 08/16] dt-bindings: reset: Add Starfive JH7100 reset bindings
In reply to Emil Renner BerthingQuoted 9 lines · Emil Renner Berthing
Add bindings for the reset controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>--- .../bindings/reset/starfive,jh7100-reset.yaml | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
Emil Renner Berthing
maintainerPATCH v3 9/16 reset: starfive-jh7100: Add StarFive JH7100 reset driver
In reply to Emil Renner BerthingAdd a driver for the StarFive JH7100 reset controller.
--- MAINTAINERS | 7 + drivers/reset/Kconfig | 7 + drivers/reset/Makefile | 1 + drivers/reset/reset-starfive-jh7100.c | 178 ++++++++++++++++++++++++++ 4 files changed, 193 insertions(+) create mode 100644 drivers/reset/reset-starfive-jh7100.c
MAINTAINERS +7 -0
@@ -17866,6 +17866,13 @@ F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml F: drivers/clk/starfive/clk-starfive-jh7100.c F: include/dt-bindings/clock/starfive-jh7100.h +STARFIVE JH7100 RESET CONTROLLER DRIVER +M: Emil Renner Berthing <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml +F: drivers/reset/reset-starfive-jh7100.c +F: include/dt-bindings/reset/starfive-jh7100.h + STATIC BRANCH/CALL M: Peter Zijlstra <[email protected]> M: Josh Poimboeuf <[email protected]>
drivers/reset/Kconfig +7 -0
@@ -224,6 +224,13 @@ config RESET_SOCFPGA This enables the reset driver for the SoCFPGA ARMv7 platforms. This driver gets initialized early during platform init calls. +config RESET_STARFIVE_JH7100 + bool "StarFive JH7100 Reset Driver" + depends on SOC_STARFIVE || COMPILE_TEST + default SOC_STARFIVE + help + This enables the reset controller driver for the StarFive JH7100 SoC. + config RESET_SUNXI bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI default ARCH_SUNXI
drivers/reset/Makefile +1 -0
@@ -29,6 +29,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o obj-$(CONFIG_RESET_SCMI) += reset-scmi.o obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o +obj-$(CONFIG_RESET_STARFIVE_JH7100) += reset-starfive-jh7100.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
drivers/reset/reset-starfive-jh7100.c new file +178 -0
178 lines not on this page · raw message
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
+Cc: Yury (bitmap expert)
Quoted 2 lines · Emil Renner Berthing
Add a driver for the StarFive JH7100 reset controller.
...
Quoted 1 line
+#define
BIT_MASK32(x)BIT((x) % 32)
Possible namespace collision.
...
Quoted 9 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Besides missing capitalization, if it sounds like bitmap, use bitmap. I have checked DT definitions and it seems you don't even need the BIT_MASK() macro,
drivers/reset/reset-starfive-jh7100.c line 50
Quoted 13 lines
drivers/reset/reset-starfive-jh7100.c new file +13
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ +BIT_MASK32 (JH7100_RST_U74 ) | +BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ +BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ +0 , +};
Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
...
Quoted 1 line
+
dev_dbg(rcdev->dev, "reset(%lu)\n", id);
These debug messages are useless since one should use ftrace facility instead,
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 24 lines · Andy Shevchenko
+Cc: Yury (bitmap expert)
Quoted 2 lines · Emil Renner Berthing
Add a driver for the StarFive JH7100 reset controller.
...
Quoted 1 line
+#define
BIT_MASK32(x)BIT((x) % 32)Possible namespace collision.
...
Quoted 9 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same.. 64bi
if it sounds like bitmap, use bitmap.
Quoted 12 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 9 lines
drivers/reset/reset-starfive-jh7100.c new file +9
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ +BIT_MASK32 (JH7100_RST_U74 ) | +BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */
Quoted 8 lines
drivers/reset/reset-starfive-jh7100.c line 50
Quoted 4 lines
drivers/reset/reset-starfive-jh7100.c new file +4
-0+BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
The problem is, that even if we could initialize this without the monstrosity in our last conversation a 64bit bitmap would still produce worse code. As it is now it's simply a 32bit load and mask with index and mask already calculated for the registers. In the status callback the mask can even be folded into the register read mask. With a 64bit bitmap you'd need to calculate new 64bit index and masks, and then conditionally shift the bits into position.
If this reflection of the 32bit registers bothers you that much we could alternatively do
static bool jh7100_reset_inverted(unsigned int idx) {
switch (idx) {
case JH7100_RST_U74:
case JH7100_RST_VP6_DRESET:
..
return false;
}
return true;
}
It'd still produce worse code, but at least it would be readable.
/Emil
Quoted 9 lines
...
Quoted 1 line
+
dev_dbg(rcdev->dev, "reset(%lu)\n", id);These debug messages are useless since one should use ftrace facility instead,
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Quoted 3 lines
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same..
Yes and there are two types of the comments, one-liners and multi-line. In multi-line you usually use proper English grammar, where capitalization means what it means. For the one-liners just choose either small letters or capital letters to start them with.
Quoted 1 line
64bi
Something is missing here.
Quoted 22 lines
if it sounds like bitmap, use bitmap.
Quoted 12 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 9 lines
drivers/reset/reset-starfive-jh7100.c new file +9
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ +BIT_MASK32 (JH7100_RST_U74 ) | +BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ Quoted 8 lines
drivers/reset/reset-starfive-jh7100.c line 50
Quoted 4 lines
drivers/reset/reset-starfive-jh7100.c new file +4
-0+BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
Quoted 7 lines
The problem is, that even if we could initialize this without the monstrosity in our last conversation a 64bit bitmap would still produce worse code. As it is now it's simply a 32bit load and mask with index and mask already calculated for the registers. In the status callback the mask can even be folded into the register read mask. With a 64bit bitmap you'd need to calculate new 64bit index and masks, and then conditionally shift the bits into position.
Why? You may use 8 byte IO (writeq() / readq() or their relaxed versions), no?
...
Quoted 1 line
If this reflection of the 32bit registers bothers you that much
What bothers me is hidden endianess issues (yeah, here it might be theoretical, but consider that somebody will look at your code and use it as the best example ever).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 16 lines · Andy Shevchenko
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same..
Yes and there are two types of the comments, one-liners and multi-line. In multi-line you usually use proper English grammar, where capitalization means what it means. For the one-liners just choose either small letters or capital letters to start them with.
That sounds reasonable, it was just that you complained about inconsistent comments in the pinctrl driver that follows the above.
Quoted 32 lines
Quoted 22 lines
if it sounds like bitmap, use bitmap.
Quoted 12 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 9 lines
drivers/reset/reset-starfive-jh7100.c new file +9
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ +BIT_MASK32 (JH7100_RST_U74 ) | +BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ Quoted 8 lines
drivers/reset/reset-starfive-jh7100.c line 50
Quoted 4 lines
drivers/reset/reset-starfive-jh7100.c new file +4
-0+BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
Quoted 7 lines
The problem is, that even if we could initialize this without the monstrosity in our last conversation a 64bit bitmap would still produce worse code. As it is now it's simply a 32bit load and mask with index and mask already calculated for the registers. In the status callback the mask can even be folded into the register read mask. With a 64bit bitmap you'd need to calculate new 64bit index and masks, and then conditionally shift the bits into position.
Why? You may use 8 byte IO (
writeq()/readq()or their relaxed versions), no?
Quoted 7 lines
...
Quoted 1 line
If this reflection of the 32bit registers bothers you that much
What bothers me is hidden endianess issues (yeah, here it might be theoretical, but consider that somebody will look at your code and use it as the best example ever).
Wouldn't endian issues be a reason to make sure we read 32bit registers with 32bit reads? Or do you expect a hypothetical big-endian StarFive SoC to also change the order of the registers?
Emil Renner Berthing
maintainerRe: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 63 lines · Emil Renner Berthing
Quoted 16 lines · Andy Shevchenko
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same..
Yes and there are two types of the comments, one-liners and multi-line. In multi-line you usually use proper English grammar, where capitalization means what it means. For the one-liners just choose either small letters or capital letters to start them with.
That sounds reasonable, it was just that you complained about inconsistent comments in the pinctrl driver that follows the above.
Quoted 34 lines
Quoted 22 lines
if it sounds like bitmap, use bitmap.
Quoted 12 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 9 lines
drivers/reset/reset-starfive-jh7100.c new file +9
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ +BIT_MASK32 (JH7100_RST_U74 ) | +BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ Quoted 8 lines
drivers/reset/reset-starfive-jh7100.c line 50
Quoted 4 lines
drivers/reset/reset-starfive-jh7100.c new file +4
-0+BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
Quoted 7 lines
The problem is, that even if we could initialize this without the monstrosity in our last conversation a 64bit bitmap would still produce worse code. As it is now it's simply a 32bit load and mask with index and mask already calculated for the registers. In the status callback the mask can even be folded into the register read mask. With a 64bit bitmap you'd need to calculate new 64bit index and masks, and then conditionally shift the bits into position.
Why? You may use 8 byte IO (
writeq()/readq()or their relaxed versions), no?Quoted 1 line
If this reflection of the 32bit registers bothers you that much
Quoted 7 lines
Quoted 3 lines
What bothers me is hidden endianess issues (yeah, here it might be theoretical, but consider that somebody will look at your code and use it as the best example ever).
Wouldn't endian issues be a reason to make sure we read 32bit registers with 32bit reads? Or do you expect a hypothetical big-endian StarFive SoC to also change the order of the registers?
Hi Andy.
I'd really like to understand your reasoning here. As far as I can tell reading 2 adjacent 32bit registers with a 64bit read as you're proposing is exactly what would cause endian issues. Eg. on little endian you'd get reg0 | reg1 << 32 whereas on big-endian you'd get reg0 << 32 | reg1.
/Emil
Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 1 line · Emil Renner Berthing
On Tue, 2 Nov 2021 at 22:17, Emil Renner Berthing <[email protected]> wrote:
...
Quoted 5 lines
I'd really like to understand your reasoning here. As far as I can tell reading 2 adjacent 32bit registers with a 64bit read as you're proposing is exactly what would cause endian issues. Eg. on little endian you'd get reg0 | reg1 << 32 whereas on big-endian you'd get reg0 << 32 | reg1.
Nope, it won't. The endianess is a property of both CPU and device.
The I/O accessors, such as readl()/writel() and iowrtieXX()/ioreadXX() are _always_ LE.
So, writeq() will properly put bits to their places in case device is LE. And most devices are LE (or should be). Of course there are cases, but then you have to specify them explicitly.
My motive here is simple as that the device is definitely a set of a few 128-bit bitmaps (in registers) and using bitmap _is_ representing hardware in the kernel. Using something else will deviate from that (maybe not too far, but still...).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 11 lines · Andy Shevchenko
Quoted 1 line · Emil Renner Berthing
On Tue, 2 Nov 2021 at 22:17, Emil Renner Berthing <[email protected]> wrote:
...
Quoted 5 lines
I'd really like to understand your reasoning here. As far as I can tell reading 2 adjacent 32bit registers with a 64bit read as you're proposing is exactly what would cause endian issues. Eg. on little endian you'd get reg0 | reg1 << 32 whereas on big-endian you'd get reg0 << 32 | reg1.
Quoted 4 lines
Nope, it won't. The endianess is a property of both CPU and device.
The I/O accessors, such as readl()/writel() and iowrtieXX()/ioreadXX() are _always_ LE.
Aha! Thanks, that's the bit I was missing.
Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
In reply to Emil Renner BerthingQuoted 12 lines · Emil Renner Berthing
Quoted 10 lines · Andy Shevchenko
+Cc: Yury (bitmap expert)
Quoted 2 lines · Emil Renner Berthing
Add a driver for the StarFive JH7100 reset controller.
...
Quoted 1 line
+#define
BIT_MASK32(x)BIT((x) % 32)
Quoted 8 lines
Quoted 8 lines
Possible namespace collision.
...
Quoted 4 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
We don't have 32-bit bitmaps. Bitmaps are always arrays of unsigned longs. On a 64-bit system this '32-bit bitmap' may be broken due to endianness issues.
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Quoted 11 lines
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same.. 64bi
if it sounds like bitmap, use bitmap.Quoted 6 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 3 lines
drivers/reset/reset-starfive-jh7100.c new file +3
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ + BIT_MASK32(JH7100_RST_U74) |
I think we have no BIT_MASK32() for a good reason. Natural alignment is always preferable.
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
drivers/reset/reset-starfive-jh7100.c new file +10
-0+BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ +BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};
Quoted 3 lines
Quoted 3 lines
Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
If you want your array to be a true bitmap, ie, all bitmap functions should work with it correctly, you'd initialize it like this:
static const unsigned long jh7100_reset_asserted[] = {
BITMAP_FROM_U64(BIT_MASK(JH7100_RST_VP6_DRESET) |
BIT_MASK(JH7100_RST_VP6_BRESET) |
BIT_MASK(JH7100_RST_HIFI4_DRESET) |
BIT_MASK(JH7100_RST_HIFI4_BRESET)),
BITMAP_FROM_U64(BIT_MASK(JH7100_RST_E24)),
}Look at lib/test_bitmap.c for example, and comment to BITMAP_FROM_U64() for internal details.
On the other hand, if you hardware has tricky requirements for bit positions, and they should depend on endianness and/or size of long in a way not compatible with bitmaps, you probably know better how to handle this. Just don't refer to your structure as a bitmap.
Thanks,
Yury
Quoted 35 lines
The problem is, that even if we could initialize this without the monstrosity in our last conversation a 64bit bitmap would still produce worse code. As it is now it's simply a 32bit load and mask with index and mask already calculated for the registers. In the status callback the mask can even be folded into the register read mask. With a 64bit bitmap you'd need to calculate new 64bit index and masks, and then conditionally shift the bits into position.
If this reflection of the 32bit registers bothers you that much we could alternatively do
static bool
jh7100_reset_inverted(unsigned int idx){
switch (idx) {
caseJH7100_RST_U74:
caseJH7100_RST_VP6_DRESET:
..return false;}
return true;
}It'd still produce worse code, but at least it would be readable.
/Emil
Quoted 9 lines
...
Quoted 1 line
+
dev_dbg(rcdev->dev, "reset(%lu)\n", id);These debug messages are useless since one should use ftrace facility instead,
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver
Quoted 65 lines · Yury Norov
Quoted 12 lines · Emil Renner Berthing
Quoted 10 lines · Andy Shevchenko
+Cc: Yury (bitmap expert)
Quoted 2 lines · Emil Renner Berthing
Add a driver for the StarFive JH7100 reset controller.
...
Quoted 1 line
+#define
BIT_MASK32(x)BIT((x) % 32)Quoted 8 lines
Quoted 8 lines
Possible namespace collision.
...
Quoted 4 lines
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.We don't have 32-bit bitmaps. Bitmaps are always arrays of unsigned longs. On a 64-bit system this '32-bit bitmap' may be broken due to endianness issues.
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */Quoted 11 lines
Quoted 1 line
Besides missing capitalization,
I'm confused. it was you who wanted all comments to capitalized the same.. 64bi
if it sounds like bitmap, use bitmap.Quoted 6 lines
I have checked DT definitions and it seems you don't even need the
BIT_MASK()macro,Quoted 3 lines
drivers/reset/reset-starfive-jh7100.c new file +3
-0+static const u32 jh7100_reset_asserted[4 ] = { +/* STATUS0 register */ + BIT_MASK32(JH7100_RST_U74) |I think we have no
BIT_MASK32()for a good reason. Natural alignment is always preferable.Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
drivers/reset/reset-starfive-jh7100.c new file +10
-0+BIT_MASK32 (JH7100_RST_VP6_DRESET ) | +BIT_MASK32 (JH7100_RST_VP6_BRESET ), +/* STATUS1 register */ +BIT_MASK32 (JH7100_RST_HIFI4_DRESET ) | +BIT_MASK32 (JH7100_RST_HIFI4_BRESET ), +/* STATUS2 register */ +BIT_MASK32 (JH7100_RST_E24 ), +/* STATUS3 register */ + 0, +};Quoted 3 lines
Quoted 3 lines
Yury, do we have any clever (clean) way to initialize a bitmap with particular bits so that it will be a constant from the beginning? If no, any suggestion what we can provide to such users?
If you want your array to be a true bitmap, ie, all bitmap functions should work with it correctly, you'd initialize it like this:
Quoted 7 lines
static const unsigned long jh7100_reset_asserted[] = {
BITMAP_FROM_U64(BIT_MASK(JH7100_RST_VP6_DRESET) |
BIT_MASK(JH7100_RST_VP6_BRESET) |
BIT_MASK(JH7100_RST_HIFI4_DRESET) |
BIT_MASK(JH7100_RST_HIFI4_BRESET)),
BITMAP_FROM_U64(BIT_MASK(JH7100_RST_E24)),
}My bad, it should be BIT_ULL_MASK.
Thanks,
Yury
Emil Renner Berthing
maintainerPATCH v3 10/16 dt-bindings: pinctrl: Add StarFive pinctrl definitions
In reply to Emil Renner BerthingAdd definitons for pins and GPIO input, output and output enable signals on the StarFive JH7100 SoC.
--- .../dt-bindings/pinctrl/pinctrl-starfive.h | 275 ++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive.h
include/dt-bindings/pinctrl/pinctrl-starfive.h new file +275 -0
@@ -0,0 +1,275 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ +/* +* Copyright (C) 2021 Emil Renner Berthing <[email protected]> +*/ + +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_H__ +#define __DT_BINDINGS_PINCTRL_STARFIVE_H__ + +#define PAD_GPIO_OFFSET 0 +#define PAD_FUNC_SHARE_OFFSET 64 +#define PAD_GPIO (x) (PAD_GPIO_OFFSET + (x)) +#define PAD_FUNC_SHARE (x) (PAD_FUNC_SHARE_OFFSET + (x)) + +/* +* GPIOMUX bits: +* | 31 - 24 | 23 - 16 | 15 - 8 | 7 | 6 | 5 - 0 | +* | dout | doen | din | dout rev | doen rev | gpio nr | +* +* dout: output signal +* doen: output enable signal +* din: optional input signal, 0xff = none +* dout rev: output signal reverse bit +* doen rev: output enable signal reverse bit +* gpio nr: gpio number, 0 - 63 +*/ +#define GPIOMUX (n, dout, doen, din) ( \ + (((dout) & 0x80000000) >> (31 - 7)) | (((dout) & 0xff) << 24) | \ + (((doen) & 0x80000000) >> (31 - 6)) | (((doen) & 0xff) << 16) | \ + (((din) & 0xff) << 8) | \ + ((n) & 0x3f)) + +#define GPO_REVERSE 0x80000000 + +#define GPO_LOW 0 +#define GPO_HIGH 1 +#define GPO_ENABLE 0 +#define GPO_DISABLE 1 +#define GPO_CLK_GMAC_PAPHYREF 2 +#define GPO_JTAG_TDO 3 +#define GPO_JTAG_TDO_OEN 4 +#define GPO_DMIC_CLK_OUT 5 +#define GPO_DSP_JTDOEN_PAD 6 +#define GPO_DSP_JTDO_PAD 7 +#define GPO_I2C0_PAD_SCK_OE 8 +#define GPO_I2C0_PAD_SCK_OEN (GPO_I2C0_PAD_SCK_OE | GPO_REVERSE) +#define GPO_I2C0_PAD_SDA_OE 9 +#define GPO_I2C0_PAD_SDA_OEN (GPO_I2C0_PAD_SDA_OE | GPO_REVERSE) +#define GPO_I2C1_PAD_SCK_OE 10 +#define GPO_I2C1_PAD_SCK_OEN (GPO_I2C1_PAD_SCK_OE | GPO_REVERSE) +#define GPO_I2C1_PAD_SDA_OE 11 +#define GPO_I2C1_PAD_SDA_OEN (GPO_I2C1_PAD_SDA_OE | GPO_REVERSE) +#define GPO_I2C2_PAD_SCK_OE 12 +#define GPO_I2C2_PAD_SCK_OEN (GPO_I2C2_PAD_SCK_OE | GPO_REVERSE) +#define GPO_I2C2_PAD_SDA_OE 13 +#define GPO_I2C2_PAD_SDA_OEN (GPO_I2C2_PAD_SDA_OE | GPO_REVERSE) +#define GPO_I2C3_PAD_SCK_OE 14 +#define GPO_I2C3_PAD_SCK_OEN (GPO_I2C3_PAD_SCK_OE | GPO_REVERSE) +#define GPO_I2C3_PAD_SDA_OE 15 +#define GPO_I2C3_PAD_SDA_OEN (GPO_I2C3_PAD_SDA_OE | GPO_REVERSE) +#define GPO_I2SRX_BCLK_OUT 16 +#define GPO_I2SRX_BCLK_OUT_OEN 17 +#define GPO_I2SRX_LRCK_OUT 18 +#define GPO_I2SRX_LRCK_OUT_OEN 19 +#define GPO_I2SRX_MCLK_OUT 20 +#define GPO_I2STX_BCLK_OUT 21 +#define GPO_I2STX_BCLK_OUT_OEN 22 +#define GPO_I2STX_LRCK_OUT 23 +#define GPO_I2STX_LRCK_OUT_OEN 24 +#define GPO_I2STX_MCLK_OUT 25 +#define GPO_I2STX_SDOUT0 26 +#define GPO_I2STX_SDOUT1 27 +#define GPO_LCD_PAD_CSM_N 28 +#define GPO_PWM_PAD_OE_N_BIT0 29 +#define GPO_PWM_PAD_OE_N_BIT1 30 +#define GPO_PWM_PAD_OE_N_BIT2 31 +#define GPO_PWM_PAD_OE_N_BIT3 32 +#define GPO_PWM_PAD_OE_N_BIT4 33 +#define GPO_PWM_PAD_OE_N_BIT5 34 +#define GPO_PWM_PAD_OE_N_BIT6 35 +#define GPO_PWM_PAD_OE_N_BIT7 36 +#define GPO_PWM_PAD_OUT_BIT0 37 +#define GPO_PWM_PAD_OUT_BIT1 38 +#define GPO_PWM_PAD_OUT_BIT2 39 +#define GPO_PWM_PAD_OUT_BIT3 40 +#define GPO_PWM_PAD_OUT_BIT4 41 +#define GPO_PWM_PAD_OUT_BIT5 42 +#define GPO_PWM_PAD_OUT_BIT6 43 +#define GPO_PWM_PAD_OUT_BIT7 44 +#define GPO_PWMDAC_LEFT_OUT 45 +#define GPO_PWMDAC_RIGHT_OUT 46 +#define GPO_QSPI_CSN1_OUT 47 +#define GPO_QSPI_CSN2_OUT 48 +#define GPO_QSPI_CSN3_OUT 49 +#define GPO_REGISTER23_SCFG_CMSENSOR_RST0 50 +#define GPO_REGISTER23_SCFG_CMSENSOR_RST1 51 +#define GPO_REGISTER32_SCFG_GMAC_PHY_RSTN 52 +#define GPO_SDIO0_PAD_CARD_POWER_EN 53 +#define GPO_SDIO0_PAD_CCLK_OUT 54 +#define GPO_SDIO0_PAD_CCMD_OE 55 +#define GPO_SDIO0_PAD_CCMD_OEN (GPO_SDIO0_PAD_CCMD_OE | GPO_REVERSE) +#define GPO_SDIO0_PAD_CCMD_OUT 56 +#define GPO_SDIO0_PAD_CDATA_OE_BIT0 57 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT0 (GPO_SDIO0_PAD_CDATA_OE_BIT0 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT1 58 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT1 (GPO_SDIO0_PAD_CDATA_OE_BIT1 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT2 59 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT2 (GPO_SDIO0_PAD_CDATA_OE_BIT2 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT3 60 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT3 (GPO_SDIO0_PAD_CDATA_OE_BIT3 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT4 61 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT4 (GPO_SDIO0_PAD_CDATA_OE_BIT4 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT5 62 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT5 (GPO_SDIO0_PAD_CDATA_OE_BIT5 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT6 63 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT6 (GPO_SDIO0_PAD_CDATA_OE_BIT6 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OE_BIT7 64 +#define GPO_SDIO0_PAD_CDATA_OEN_BIT7 (GPO_SDIO0_PAD_CDATA_OE_BIT7 | GPO_REVERSE) +#define GPO_SDIO0_PAD_CDATA_OUT_BIT0 65 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT1 66 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT2 67 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT3 68 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT4 69 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT5 70 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT6 71 +#define GPO_SDIO0_PAD_CDATA_OUT_BIT7 72 +#define GPO_SDIO0_PAD_RST_N 73 +#define GPO_SDIO1_PAD_CARD_POWER_EN 74 +#define GPO_SDIO1_PAD_CCLK_OUT 75 +#define GPO_SDIO1_PAD_CCMD_OE 76 +#define GPO_SDIO1_PAD_CCMD_OEN (GPO_SDIO1_PAD_CCMD_OE | GPO_REVERSE) +#define GPO_SDIO1_PAD_CCMD_OUT 77 +#define GPO_SDIO1_PAD_CDATA_OE_BIT0 78 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT0 (GPO_SDIO1_PAD_CDATA_OE_BIT0 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT1 79 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT1 (GPO_SDIO1_PAD_CDATA_OE_BIT1 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT2 80 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT2 (GPO_SDIO1_PAD_CDATA_OE_BIT2 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT3 81 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT3 (GPO_SDIO1_PAD_CDATA_OE_BIT3 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT4 82 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT4 (GPO_SDIO1_PAD_CDATA_OE_BIT4 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT5 83 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT5 (GPO_SDIO1_PAD_CDATA_OE_BIT5 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT6 84 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT6 (GPO_SDIO1_PAD_CDATA_OE_BIT6 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OE_BIT7 85 +#define GPO_SDIO1_PAD_CDATA_OEN_BIT7 (GPO_SDIO1_PAD_CDATA_OE_BIT7 | GPO_REVERSE) +#define GPO_SDIO1_PAD_CDATA_OUT_BIT0 86 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT1 87 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT2 88 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT3 89 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT4 90 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT5 91 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT6 92 +#define GPO_SDIO1_PAD_CDATA_OUT_BIT7 93 +#define GPO_SDIO1_PAD_RST_N 94 +#define GPO_SPDIF_TX_SDOUT 95 +#define GPO_SPDIF_TX_SDOUT_OEN 96 +#define GPO_SPI0_PAD_OE_N 97 +#define GPO_SPI0_PAD_SCK_OUT 98 +#define GPO_SPI0_PAD_SS_0_N 99 +#define GPO_SPI0_PAD_SS_1_N 100 +#define GPO_SPI0_PAD_TXD 101 +#define GPO_SPI1_PAD_OE_N 102 +#define GPO_SPI1_PAD_SCK_OUT 103 +#define GPO_SPI1_PAD_SS_0_N 104 +#define GPO_SPI1_PAD_SS_1_N 105 +#define GPO_SPI1_PAD_TXD 106 +#define GPO_SPI2_PAD_OE_N 107 +#define GPO_SPI2_PAD_SCK_OUT 108 +#define GPO_SPI2_PAD_SS_0_N 109 +#define GPO_SPI2_PAD_SS_1_N 110 +#define GPO_SPI2_PAD_TXD 111 +#define GPO_SPI2AHB_PAD_OE_N_BIT0 112 +#define GPO_SPI2AHB_PAD_OE_N_BIT1 113 +#define GPO_SPI2AHB_PAD_OE_N_BIT2 114 +#define GPO_SPI2AHB_PAD_OE_N_BIT3 115 +#define GPO_SPI2AHB_PAD_TXD_BIT0 116 +#define GPO_SPI2AHB_PAD_TXD_BIT1 117 +#define GPO_SPI2AHB_PAD_TXD_BIT2 118 +#define GPO_SPI2AHB_PAD_TXD_BIT3 119 +#define GPO_SPI3_PAD_OE_N 120 +#define GPO_SPI3_PAD_SCK_OUT 121 +#define GPO_SPI3_PAD_SS_0_N 122 +#define GPO_SPI3_PAD_SS_1_N 123 +#define GPO_SPI3_PAD_TXD 124 +#define GPO_UART0_PAD_DTRN 125 +#define GPO_UART0_PAD_RTSN 126 +#define GPO_UART0_PAD_SOUT 127 +#define GPO_UART1_PAD_SOUT 128 +#define GPO_UART2_PAD_DTR_N 129 +#define GPO_UART2_PAD_RTS_N 130 +#define GPO_UART2_PAD_SOUT 131 +#define GPO_UART3_PAD_SOUT 132 +#define GPO_USB_DRV_BUS 133 + +#define GPI_CPU_JTAG_TCK 0 +#define GPI_CPU_JTAG_TDI 1 +#define GPI_CPU_JTAG_TMS 2 +#define GPI_CPU_JTAG_TRST 3 +#define GPI_DMIC_SDIN_BIT0 4 +#define GPI_DMIC_SDIN_BIT1 5 +#define GPI_DSP_JTCK_PAD 6 +#define GPI_DSP_JTDI_PAD 7 +#define GPI_DSP_JTMS_PAD 8 +#define GPI_DSP_TRST_PAD 9 +#define GPI_I2C0_PAD_SCK_IN 10 +#define GPI_I2C0_PAD_SDA_IN 11 +#define GPI_I2C1_PAD_SCK_IN 12 +#define GPI_I2C1_PAD_SDA_IN 13 +#define GPI_I2C2_PAD_SCK_IN 14 +#define GPI_I2C2_PAD_SDA_IN 15 +#define GPI_I2C3_PAD_SCK_IN 16 +#define GPI_I2C3_PAD_SDA_IN 17 +#define GPI_I2SRX_BCLK_IN 18 +#define GPI_I2SRX_LRCK_IN 19 +#define GPI_I2SRX_SDIN_BIT0 20 +#define GPI_I2SRX_SDIN_BIT1 21 +#define GPI_I2SRX_SDIN_BIT2 22 +#define GPI_I2STX_BCLK_IN 23 +#define GPI_I2STX_LRCK_IN 24 +#define GPI_SDIO0_PAD_CARD_DETECT_N 25 +#define GPI_SDIO0_PAD_CARD_WRITE_PRT 26 +#define GPI_SDIO0_PAD_CCMD_IN 27 +#define GPI_SDIO0_PAD_CDATA_IN_BIT0 28 +#define GPI_SDIO0_PAD_CDATA_IN_BIT1 29 +#define GPI_SDIO0_PAD_CDATA_IN_BIT2 30 +#define GPI_SDIO0_PAD_CDATA_IN_BIT3 31 +#define GPI_SDIO0_PAD_CDATA_IN_BIT4 32 +#define GPI_SDIO0_PAD_CDATA_IN_BIT5 33 +#define GPI_SDIO0_PAD_CDATA_IN_BIT6 34 +#define GPI_SDIO0_PAD_CDATA_IN_BIT7 35 +#define GPI_SDIO1_PAD_CARD_DETECT_N 36 +#define GPI_SDIO1_PAD_CARD_WRITE_PRT 37 +#define GPI_SDIO1_PAD_CCMD_IN 38 +#define GPI_SDIO1_PAD_CDATA_IN_BIT0 39 +#define GPI_SDIO1_PAD_CDATA_IN_BIT1 40 +#define GPI_SDIO1_PAD_CDATA_IN_BIT2 41 +#define GPI_SDIO1_PAD_CDATA_IN_BIT3 42 +#define GPI_SDIO1_PAD_CDATA_IN_BIT4 43 +#define GPI_SDIO1_PAD_CDATA_IN_BIT5 44 +#define GPI_SDIO1_PAD_CDATA_IN_BIT6 45 +#define GPI_SDIO1_PAD_CDATA_IN_BIT7 46 +#define GPI_SPDIF_RX_SDIN 47 +#define GPI_SPI0_PAD_RXD 48 +#define GPI_SPI0_PAD_SS_IN_N 49 +#define GPI_SPI1_PAD_RXD 50 +#define GPI_SPI1_PAD_SS_IN_N 51 +#define GPI_SPI2_PAD_RXD 52 +#define GPI_SPI2_PAD_SS_IN_N 53 +#define GPI_SPI2AHB_PAD_RXD_BIT0 54 +#define GPI_SPI2AHB_PAD_RXD_BIT1 55 +#define GPI_SPI2AHB_PAD_RXD_BIT2 56 +#define GPI_SPI2AHB_PAD_RXD_BIT3 57 +#define GPI_SPI2AHB_PAD_SS_N 58 +#define GPI_SPI2AHB_SLV_SCLKIN 59 +#define GPI_SPI3_PAD_RXD 60 +#define GPI_SPI3_PAD_SS_IN_N 61 +#define GPI_UART0_PAD_CTSN 62 +#define GPI_UART0_PAD_DCDN 63 +#define GPI_UART0_PAD_DSRN 64 +#define GPI_UART0_PAD_RIN 65 +#define GPI_UART0_PAD_SIN 66 +#define GPI_UART1_PAD_SIN 67 +#define GPI_UART2_PAD_CTS_N 68 +#define GPI_UART2_PAD_DCD_N 69 +#define GPI_UART2_PAD_DSR_N 70 +#define GPI_UART2_PAD_RI_N 71 +#define GPI_UART2_PAD_SIN 72 +#define GPI_UART3_PAD_SIN 73 +#define GPI_USB_OVER_CURRENT 74 + +#define GPI_NONE 0xff + +#endif /* __DT_BINDINGS_PINCTRL_STARFIVE_H__ */
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 10/16] dt-bindings: pinctrl: Add StarFive pinctrl definitions
Quoted 9 lines · Emil Renner Berthing
Add definitons for pins and GPIO input, output and output enable signals on the StarFive JH7100 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>--- .../dt-bindings/pinctrl/pinctrl-starfive.h | 275 ++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive.h
Emil Renner Berthing
maintainerPATCH v3 11/16 dt-bindings: pinctrl: Add StarFive JH7100 bindings
In reply to Emil Renner BerthingAdd bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
---
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
.../pinctrl/starfive,jh7100-pinctrl.yaml | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml new file +307 -0
@@ -0,0 +1,307 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7100-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7100 Pin Controller Device Tree Bindings + +description: | + Bindings for the JH7100 RISC-V SoC from StarFive Ltd. + + Out of the SoC's many pins only the ones named PAD_GPIO[0] to PAD_GPIO[63] + and PAD_FUNC_SHARE[0] to PAD_FUNC_SHARE[141] can be multiplexed and have + configurable bias, drive strength, schmitt trigger etc. The SoC has an + interesting 2-layered approach to pin muxing best illustrated by the diagram + below. + + Signal group 0, 1, ... or 6 + ___|___ + | | + LCD output -----------------| | + CMOS Camera interface ------| |--- PAD_GPIO[0] + Ethernet PHY interface -----| MUX |--- PAD_GPIO[1] + ... | | ... + | |--- PAD_GPIO[63] + -------- GPIO0 ------------| | + | -------|-- GPIO1 --------| |--- PAD_FUNC_SHARE[0] + | | | | | |--- PAD_FUNC_SHARE[1] + | | | | ... | | ... + | | | | | |--- PAD_FUNC_SHARE[141] + | | -----|---|-- GPIO63 ---| | + | | | | | | ------- + UART0 UART1 -- + + + The big MUX in the diagram only has 7 different ways of mapping peripherals + on the left to pins on the right. StarFive calls the 7 configurations "signal + groups". + However some peripherals have their I/O go through the 64 "GPIOs". The + diagram only shows UART0 and UART1, but this also includes a number of other + UARTs, I2Cs, SPIs, PWMs etc. All these peripherals are connected to all 64 + GPIOs such that any GPIO can be set up to be controlled by any of the + peripherals. + Note that signal group 0 doesn't map any of the GPIOs to pins, and only + signal group 1 maps the GPIOs to the pins named PAD_GPIO[0] to PAD_GPIO[63]. + +maintainers: + - Emil Renner Berthing <[email protected]> + - Drew Fustini <[email protected]> + +properties: + compatible: + const: starfive,jh7100-pinctrl + + reg: + minItems: 2 + maxItems: 2 + + reg-names: + items: + - const: gpio + - const: padctl + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + gpio-controller: true + + "#gpio-cells": + const: 2 + + interrupts: + maxItems: 1 + description: The GPIO parent interrupt. + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + starfive,signal-group: + description: | + Select one of the 7 signal groups. If this property is not set it + defaults to the configuration already chosen by the earlier boot stages. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3, 4, 5, 6] + +required: + - compatible + - reg + - reg-names + - clocks + - gpio-controller + - "#gpio-cells" + - interrupts + - interrupt-controller + - "#interrupt-cells" + +patternProperties: + '-[0-9]+$': + type: object + patternProperties: + '-pins$': + type: object + description: | + A pinctrl node should contain at least one subnode representing the + pinctrl groups available on the machine. Each subnode will list the + pins it needs, and how they should be configured, with regard to + muxer configuration, bias, input enable/disable, input schmitt + trigger enable/disable, slew-rate and drive strength. + $ref: "/schemas/pinctrl/pincfg-node.yaml" + + properties: + pins: + description: | + The list of pin identifiers that properties in the node apply to. + This should be set using either the PAD_GPIO or PAD_FUNC_SHARE + macros. + Either this or "pinmux" has to be specified, but not both. + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pins" + + pinmux: + description: | + The list of GPIOs and their mux settings that properties in the + node apply to. This should be set using the GPIOMUX macro. + Either this or "pins" has to be specified, but not both. + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux" + + bias-disable: true + + bias-pull-up: + type: boolean + + bias-pull-down: + type: boolean + + drive-strength: + enum: [ 14, 21, 28, 35, 42, 49, 56, 63 ] + + input-enable: true + + input-disable: true + + input-schmitt-enable: true + + input-schmitt-disable: true + + slew-rate: + maximum: 7 + + starfive,strong-pull-up: + description: enable strong pull-up. + type: boolean + + additionalProperties: false + + additionalProperties: false + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/starfive-jh7100.h> + #include <dt-bindings/reset/starfive-jh7100.h> + #include <dt-bindings/pinctrl/pinctrl-starfive.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + pinctrl@11910000 { + compatible = "starfive,jh7100-pinctrl"; + reg = <0x0 0x11910000 0x0 0x10000>, + <0x0 0x11858000 0x0 0x1000>; + reg-names = "gpio", "padctl"; + clocks = <&clkgen JH7100_CLK_GPIO_APB>; + resets = <&clkgen JH7100_RSTN_GPIO_APB>; + interrupts = <32>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + starfive,signal-group = <6>; + + gmac_pins_default: gmac-0 { + gtxclk-pins { + pins = <PAD_FUNC_SHARE(115)>; + bias-pull-up; + drive-strength = <35>; + input-enable; + input-schmitt-enable; + slew-rate = <0>; + }; + miitxclk-pins { + pins = <PAD_FUNC_SHARE(116)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <0>; + }; + tx-pins { + pins = <PAD_FUNC_SHARE(117)>, + <PAD_FUNC_SHARE(119)>, + <PAD_FUNC_SHARE(120)>, + <PAD_FUNC_SHARE(121)>, + <PAD_FUNC_SHARE(122)>, + <PAD_FUNC_SHARE(123)>, + <PAD_FUNC_SHARE(124)>, + <PAD_FUNC_SHARE(125)>, + <PAD_FUNC_SHARE(126)>; + bias-disable; + drive-strength = <35>; + input-disable; + input-schmitt-disable; + slew-rate = <0>; + }; + rxclk-pins { + pins = <PAD_FUNC_SHARE(127)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <6>; + }; + rxer-pins { + pins = <PAD_FUNC_SHARE(129)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <0>; + }; + rx-pins { + pins = <PAD_FUNC_SHARE(128)>, + <PAD_FUNC_SHARE(130)>, + <PAD_FUNC_SHARE(131)>, + <PAD_FUNC_SHARE(132)>, + <PAD_FUNC_SHARE(133)>, + <PAD_FUNC_SHARE(134)>, + <PAD_FUNC_SHARE(135)>, + <PAD_FUNC_SHARE(136)>, + <PAD_FUNC_SHARE(137)>, + <PAD_FUNC_SHARE(138)>, + <PAD_FUNC_SHARE(139)>, + <PAD_FUNC_SHARE(140)>, + <PAD_FUNC_SHARE(141)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-enable; + slew-rate = <0>; + }; + }; + + i2c0_pins_default: i2c0-0 { + i2c-pins { + pinmux = <GPIOMUX(62, GPO_LOW, + GPO_I2C0_PAD_SCK_OEN, + GPI_I2C0_PAD_SCK_IN)>, + <GPIOMUX(61, GPO_LOW, + GPO_I2C0_PAD_SDA_OEN, + GPI_I2C0_PAD_SDA_IN)>; + bias-disable; /* external pull-up */ + input-enable; + input-schmitt-enable; + }; + }; + + uart3_pins_default: uart3-0 { + rx-pins { + pinmux = <GPIOMUX(13, GPO_LOW, GPO_DISABLE, + GPI_UART3_PAD_SIN)>; + bias-pull-up; + input-enable; + input-schmitt-enable; + }; + tx-pins { + pinmux = <GPIOMUX(14, GPO_UART3_PAD_SOUT, + GPO_ENABLE, GPI_NONE)>; + bias-disable; + input-disable; + input-schmitt-disable; + }; + }; + }; + + gmac { + pinctrl-0 = <&gmac_pins_default>; + pinctrl-names = "default"; + }; + + i2c0 { + pinctrl-0 = <&i2c0_pins_default>; + pinctrl-names = "default"; + }; + + uart3 { + pinctrl-0 = <&uart3_pins_default>; + pinctrl-names = "default"; + }; + }; + +...
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Rob Herring
maintainerRe: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
Quoted 6 lines · Emil Renner Berthing
Add bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>---
Quoted 9 lines
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
.../pinctrl/starfive,jh7100-pinctrl.yaml | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.example.dts:19:18: fatal error: dt-bindings/clock/starfive-jh7100.h: No such file or directory
19 | #include <dt-bindings/clock/starfive-jh7100.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1549835
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.
Emil Renner Berthing
maintainerRe: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
Quoted 26 lines · Rob Herring
Quoted 6 lines · Emil Renner Berthing
Add bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>---
Quoted 9 lines
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
.../pinctrl/starfive,jh7100-pinctrl.yaml | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yamlMy bot found errors running 'make DT_CHECKER_FLAGS=-m
dt_binding_check' on your patch (DT_CHECKER_FLAGSis new in v5.13):yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.example.dts:19:18: fatal error:dt-bindings/clock/starfive-jh7100.h: No such file or directory
19 | #include <dt-bindings/clock/starfive-jh7100.h>| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 8 lines
compilation terminated.
make[1]: *** [scripts/Makefile.lib:385:Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441:dt_binding_check] Error 2doc reference errors (make refcheckdocs):
Hi Rob.
It seems like your bot didn't add the clock header because the patch already has an Acked-by from you.
/Emil
Re: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
In reply to Emil Renner BerthingQuoted 10 lines · Emil Renner Berthing
Add bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>---
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
This looks perfectly acceptable to me:
As it appears to have some cross dependencies I assume it will be merged through the SoC tree?
Yours,
Linus Walleij
Emil Renner Berthing
maintainerRe: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
Quoted 7 lines · Linus Walleij
Quoted 6 lines · Emil Renner Berthing
Add bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>---
Quoted 7 lines
Quoted 4 lines
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
This looks perfectly acceptable to me:
Reviewed-by Linus Walleij <[email protected]>
Thank you.
Quoted 2 lines
As it appears to have some cross dependencies I assume it will be merged through the SoC tree?
I don't know. I've never tried this before, so whatever is easiest I guess. Do I do anything special other than cc'ing [email protected] for v4 to make that happen?
/Emil
Re: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
Quoted 8 lines · Emil Renner Berthing
Quoted 2 lines · Linus Walleij
As it appears to have some cross dependencies I assume it will be merged through the SoC tree?
I don't know. I've never tried this before, so whatever is easiest I guess. Do I do anything special other than cc'ing [email protected] for v4 to make that happen?
You create a pull request with git request-pull and sent it to [email protected] and linux-arm-kernel on Cc that should work fine and it appears here:
https://patchwork.kernel.org/project/linux-soc/list/
Yours,
Linus Walleij
Emil Renner Berthing
maintainerRe: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
Quoted 6 lines · Linus Walleij
Quoted 5 lines · Emil Renner Berthing
Quoted 2 lines · Linus Walleij
As it appears to have some cross dependencies I assume it will be merged through the SoC tree?
Quoted 8 lines
Quoted 3 lines
I don't know. I've never tried this before, so whatever is easiest I guess. Do I do anything special other than cc'ing [email protected] for v4 to make that happen?
You create a pull request with git request-pull and sent it to [email protected] and linux-arm-kernel on Cc that should work fine and it appears here:
https://patchwork.kernel.org/project/linux-soc/list/
Thanks. I'm just waiting for Palmer to ack the riscv bits then I'll do that.
/Emil
Re: [PATCH v3 11/16] dt-bindings: pinctrl: Add StarFive JH7100 bindings
In reply to Emil Renner BerthingQuoted 6 lines · Emil Renner Berthing
Add bindings for the GPIO/pin controller on the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
Signed-off-by Emil Renner Berthing <[email protected]>---
Quoted 9 lines
@Linus: I'm really struggling to find a good way to describe how pin muxing works on the JH7100. As you can see I've now resorted to ascii-art to try to explain it, but please let me know if it's still unclear.
.../pinctrl/starfive,jh7100-pinctrl.yaml | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
Emil Renner Berthing
maintainerPATCH v3 12/16 pinctrl: starfive: Add pinctrl driver for StarFive SoCs
In reply to Emil Renner BerthingAdd a combined pinctrl and GPIO driver for the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC, which is said to feature only minor changes to these pinctrl/GPIO parts.
For each "GPIO" there are two registers for configuring the output and output enable signals which may come from other peripherals. Among these are two special signals that are constant 0 and constant 1 respectively. Controlling the GPIOs from software is done by choosing one of these signals. In other words the same registers are used for both pin muxing and controlling the GPIOs, which makes it easier to combine the pinctrl and GPIO driver in one.
I wrote the pinconf and pinmux parts, but the GPIO part of the code is based on the GPIO driver in the vendor tree written by Huan Feng with cleanups and fixes by Drew and me.
--- MAINTAINERS | 8 + drivers/pinctrl/Kconfig | 17 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-starfive.c | 1353 ++++++++++++++++++++++++++++ 4 files changed, 1379 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-starfive.c
MAINTAINERS +8 -0
@@ -17866,6 +17866,14 @@ F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml F: drivers/clk/starfive/clk-starfive-jh7100.c F: include/dt-bindings/clock/starfive-jh7100.h +STARFIVE JH7100 PINCTRL DRIVER +M: Emil Renner Berthing <[email protected]> +L: [email protected] +S: Maintained +F: Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml +F: drivers/pinctrl/pinctrl-starfive.c +F: include/dt-bindings/pinctrl/pinctrl-starfive.h + STARFIVE JH7100 RESET CONTROLLER DRIVER M: Emil Renner Berthing <[email protected]> S: Maintained
drivers/pinctrl/Kconfig +17 -0
@@ -265,6 +265,23 @@ config PINCTRL_ST select PINCONF select GPIOLIB_IRQCHIP +config PINCTRL_STARFIVE + tristate "Pinctrl and GPIO driver for the StarFive JH7100 SoC" + depends on SOC_STARFIVE || COMPILE_TEST + depends on OF + default SOC_STARFIVE + select GENERIC_PINCTRL_GROUPS + select GENERIC_PINMUX_FUNCTIONS + select GENERIC_PINCONF + select GPIOLIB + select GPIOLIB_IRQCHIP + select OF_GPIO + help + Say yes here to support pin control on the StarFive JH7100 SoC. + This also provides an interface to the GPIO pins not used by other + peripherals supporting inputs, outputs, configuring pull-up/pull-down + and interrupts on input changes. + config PINCTRL_STMFX tristate "STMicroelectronics STMFX GPIO expander pinctrl driver" depends on I2C
drivers/pinctrl/Makefile +1 -0
@@ -38,6 +38,7 @@ obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o obj-$(CONFIG_PINCTRL_TB10X) += pinctrl-tb10x.o obj-$(CONFIG_PINCTRL_ST) += pinctrl-st.o +obj-$(CONFIG_PINCTRL_STARFIVE) += pinctrl-starfive.o obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o obj-$(CONFIG_PINCTRL_ZYNQ) += pinctrl-zynq.o obj-$(CONFIG_PINCTRL_ZYNQMP) += pinctrl-zynqmp.o
drivers/pinctrl/pinctrl-starfive.c new file +1353 -0
1,353 lines not on this page · raw message
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 16 lines · Emil Renner Berthing
Add a combined pinctrl and GPIO driver for the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC, which is said to feature only minor changes to these pinctrl/GPIO parts.
For each "GPIO" there are two registers for configuring the output and output enable signals which may come from other peripherals. Among these are two special signals that are constant 0 and constant 1 respectively. Controlling the GPIOs from software is done by choosing one of these signals. In other words the same registers are used for both pin muxing and controlling the GPIOs, which makes it easier to combine the pinctrl and GPIO driver in one.
I wrote the pinconf and pinmux parts, but the GPIO part of the code is based on the GPIO driver in the vendor tree written by Huan Feng with cleanups and fixes by Drew and me.
...
Quoted 1 line
+ depends on OF
So this descreases test coverage.
Linus, can we provide a necessary stub so we may drop this dependency?
...
drivers/pinctrl/pinctrl-starfive.c line 216
Quoted 5 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+static inline struct device *starfive_dev (const struct starfive_pinctrl *sfp) +{ +return sfp->gc.parent; +} +
This seems useless helper. You may do what it's doing just in place. It will save 5 LOCs.
...
drivers/pinctrl/pinctrl-starfive.c line 463
Quoted 11 lines
drivers/pinctrl/pinctrl-starfive.c new file +10
-0+static void starfive_pin_dbg_show (struct pinctrl_dev *pctldev, +struct seq_file *s, +unsigned int pin) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +unsigned int gpio =starfive_pin_to_gpio (sfp, pin); +void __iomem *reg; +u32 dout, doen; +if (gpio >=NR_GPIOS ) +return ;
Dead code?
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+ reg = sfp->base +GPON_DOUT_CFG +8 * gpio; + dout =readl_relaxed (reg +0x000 ); + doen =readl_relaxed (reg +0x004 ); + +seq_printf (s,"dout=%lu%s doen=%lu%s" , + dout &GENMASK (7 ,0 ), (dout &BIT (31 )) ?"r" :"" , + doen &GENMASK (7 ,0 ), (doen &BIT (31 )) ?"r" :"" ); +}
...
drivers/pinctrl/pinctrl-starfive.c line 489
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +struct device *dev =starfive_dev (sfp); +const char **pgnames; +struct pinctrl_map *map; +struct device_node *child; +const char *grpname; +int *pins; +u32 *pinmux;
Reversed xmas tree order?
Quoted 3 lines
drivers/pinctrl/pinctrl-starfive.c new file +3
-0+int nmaps; +int ngroups; +int ret;
...
Quoted 81 lines
drivers/pinctrl/pinctrl-starfive.c new file +81
-0+static int starfive_pinconf_group_set (struct pinctrl_dev *pctldev, +unsigned int gsel, +unsigned long *configs, +unsigned int num_configs) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +const struct group_desc *group; +u16 mask, value; +int i; + + group =pinctrl_generic_get_group (pctldev, gsel); +if (!group) +return -EINVAL ; + + mask =0 ; + value =0 ; +for (i =0 ; i < num_configs; i++) { +int param =pinconf_to_config_param (configs[i]); +u32 arg =pinconf_to_config_argument (configs[i]); + +switch (param) { +case PIN_CONFIG_BIAS_DISABLE : + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_DISABLE ; +break ; +case PIN_CONFIG_BIAS_PULL_DOWN : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_PULL_DOWN ; +break ; +case PIN_CONFIG_BIAS_PULL_UP : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = value & ~PAD_BIAS_MASK ; +break ; +case PIN_CONFIG_DRIVE_STRENGTH : + mask |=PAD_DRIVE_STRENGTH_MASK ; + value = (value & ~PAD_DRIVE_STRENGTH_MASK ) | +starfive_drive_strength_from_max_mA (arg); +break ; +case PIN_CONFIG_INPUT_ENABLE : + mask |=PAD_INPUT_ENABLE ; +if (arg) + value |=PAD_INPUT_ENABLE ; +else + value &= ~PAD_INPUT_ENABLE ; +break ; +case PIN_CONFIG_INPUT_SCHMITT_ENABLE : + mask |=PAD_INPUT_SCHMITT_ENABLE ; +if (arg) + value |=PAD_INPUT_SCHMITT_ENABLE ; +else + value &= ~PAD_INPUT_SCHMITT_ENABLE ; +break ; +case PIN_CONFIG_SLEW_RATE : + mask |=PAD_SLEW_RATE_MASK ; + value = (value & ~PAD_SLEW_RATE_MASK ) | + ((arg <<PAD_SLEW_RATE_POS ) &PAD_SLEW_RATE_MASK ); +break ; +case PIN_CONFIG_STARFIVE_STRONG_PULL_UP : +if (arg) { + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) | +PAD_BIAS_STRONG_PULL_UP ; + }else { + mask |=PAD_BIAS_STRONG_PULL_UP ; + value = value & ~PAD_BIAS_STRONG_PULL_UP ; + } +break ; +default : +return -ENOTSUPP ; + } + } + +for (i =0 ; i < group->num_pins; i++) +starfive_padctl_rmw (sfp, group->pins[i], mask, value); + +return 0 ; +}
...
drivers/pinctrl/pinctrl-starfive.c line 933
Quoted 7 lines
drivers/pinctrl/pinctrl-starfive.c new file +7
-0+static int starfive_gpio_get_direction (struct gpio_chip *gc,unsigned int gpio) +{ + struct starfive_pinctrl *sfp = container_of(gc, struct starfive_pinctrl, gc); +void __iomem *doen = sfp->base +GPON_DOEN_CFG +8 * gpio; + +/* return GPIO_LINE_DIRECTION_OUT (0) only if doen == GPO_ENABLE (0) */ +return readl_relaxed (doen) !=GPO_ENABLE ;
I believe the idea was to return the predefined values for the direction.
Quoted 1 line
+}
...
drivers/pinctrl/pinctrl-starfive.c line 1126
Quoted 13 lines
drivers/pinctrl/pinctrl-starfive.c new file +13
-0+static int starfive_irq_set_type (struct irq_data *d,unsigned int trigger) +{ +struct starfive_pinctrl *sfp =starfive_from_irq_data (d); +irq_hw_number_t gpio =irqd_to_hwirq (d); +void __iomem *base = sfp->base +4 * (gpio /32 ); +u32 mask =BIT (gpio %32 ); +u32 irq_type, edge_both, polarity; +unsigned long flags; + +if (trigger &IRQ_TYPE_EDGE_BOTH ) +irq_set_handler_locked (d, handle_edge_irq); +else if (trigger &IRQ_TYPE_LEVEL_MASK ) +irq_set_handler_locked (d, handle_level_irq);
Usually we don't assign this twice, so it should be after the switch.
Quoted 19 lines
drivers/pinctrl/pinctrl-starfive.c new file +19
-0+switch (trigger) { +case IRQ_TYPE_EDGE_RISING : + irq_type = mask;/* 1: edge triggered */ + edge_both =0 ;/* 0: single edge */ + polarity = mask;/* 1: rising edge */ +break ; +case IRQ_TYPE_EDGE_FALLING : + irq_type = mask;/* 1: edge triggered */ + edge_both =0 ;/* 0: single edge */ + polarity =0 ;/* 0: falling edge */ +break ; +case IRQ_TYPE_EDGE_BOTH : + irq_type = mask;/* 1: edge triggered */ + edge_both = mask;/* 1: both edges */ + polarity =0 ;/* 0: ignored */ +break ; +case IRQ_TYPE_LEVEL_HIGH : + irq_type =0 ;/* 0: level triggered */ + edge_both =0 ;/* 0: ignored */
drivers/pinctrl/pinctrl-starfive.c line 1155
Quoted 10 lines
drivers/pinctrl/pinctrl-starfive.c new file +9
-0+ polarity = mask;/* 1: high level */ +break ; +case IRQ_TYPE_LEVEL_LOW : + irq_type =0 ;/* 0: level triggered */ + edge_both =0 ;/* 0: ignored */ + polarity =0 ;/* 0: low level */ +break ; +default : +irq_set_handler_locked (d, handle_bad_irq);
Why? You have it already in ->probe(), what's the point?
Quoted 13 lines
drivers/pinctrl/pinctrl-starfive.c new file +13
-0+return -EINVAL ; + } + +raw_spin_lock_irqsave (&sfp->lock, flags); + irq_type |=readl_relaxed (base +GPIOIS ) & ~mask; +writel_relaxed (irq_type, base +GPIOIS ); + edge_both |=readl_relaxed (base +GPIOIBE ) & ~mask; +writel_relaxed (edge_both, base +GPIOIBE ); + polarity |=readl_relaxed (base +GPIOIEV ) & ~mask; +writel_relaxed (polarity, base +GPIOIEV ); +raw_spin_unlock_irqrestore (&sfp->lock, flags); +return 0 ; +}
...
drivers/pinctrl/pinctrl-starfive.c line 1265
Quoted 6 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+ ret =reset_control_deassert (rst); +if (ret) +return dev_err_probe (dev, ret,"could not deassert resetd\n" ); + ret =devm_pinctrl_register_and_init (dev, &starfive_desc, sfp, &sfp->pctl); +if (ret)
I don't see who will assert reset here.
Quoted 1 line
+ return
dev_err_probe(dev, ret, "could not register pinctrl driver\n");
...
Quoted 7 lines
drivers/pinctrl/pinctrl-starfive.c new file +7
-0+switch (value) { +case 0 : + sfp->gpios.pin_base =PAD_INVALID_GPIO ; + goto done; +case 1 : + sfp->gpios.pin_base =PAD_GPIO (0 ); +break ;
drivers/pinctrl/pinctrl-starfive.c line 1291
Quoted 10 lines
drivers/pinctrl/pinctrl-starfive.c new file +10
-0+case 2 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (72 ); +break ; +case 3 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (70 ); +break ; +case 4 :case 5 :case 6 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (0 ); +break ; +default :
Ditto.
Quoted 2 lines
(quoted diff) +2
-0+ return dev_err_probe(dev, -EINVAL, "invalid signal group %u\n", value); + }
...
drivers/pinctrl/pinctrl-starfive.c line 1329
Quoted 2 lines
drivers/pinctrl/pinctrl-starfive.c new file +2
-0+ ret =devm_gpiochip_add_data (dev, &sfp->gc, sfp); +if (ret)
Ditto.
drivers/pinctrl/pinctrl-starfive.c line 1333
Quoted 4 lines
drivers/pinctrl/pinctrl-starfive.c new file +4
-0+return dev_err_probe (dev, ret,"could not register gpiochip\n" ); + +done: +return pinctrl_enable (sfp->pctl);
Ditto.
And better to use label name like followingout_pinctrl_enable:
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 73 lines
Quoted 73 lines
drivers/pinctrl/pinctrl-starfive.c new file +73
-0+static int starfive_pinconf_group_set (struct pinctrl_dev *pctldev, +unsigned int gsel, +unsigned long *configs, +unsigned int num_configs) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +const struct group_desc *group; +u16 mask, value; +int i; + + group =pinctrl_generic_get_group (pctldev, gsel); +if (!group) +return -EINVAL ; + + mask =0 ; + value =0 ; +for (i =0 ; i < num_configs; i++) { +int param =pinconf_to_config_param (configs[i]); +u32 arg =pinconf_to_config_argument (configs[i]); + +switch (param) { +case PIN_CONFIG_BIAS_DISABLE : + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_DISABLE ; +break ; +case PIN_CONFIG_BIAS_PULL_DOWN : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_PULL_DOWN ; +break ; +case PIN_CONFIG_BIAS_PULL_UP : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = value & ~PAD_BIAS_MASK ; +break ; +case PIN_CONFIG_DRIVE_STRENGTH : + mask |=PAD_DRIVE_STRENGTH_MASK ; + value = (value & ~PAD_DRIVE_STRENGTH_MASK ) | +starfive_drive_strength_from_max_mA (arg); +break ; +case PIN_CONFIG_INPUT_ENABLE : + mask |=PAD_INPUT_ENABLE ; +if (arg) + value |=PAD_INPUT_ENABLE ; +else + value &= ~PAD_INPUT_ENABLE ; +break ; +case PIN_CONFIG_INPUT_SCHMITT_ENABLE : + mask |=PAD_INPUT_SCHMITT_ENABLE ; +if (arg) + value |=PAD_INPUT_SCHMITT_ENABLE ; +else + value &= ~PAD_INPUT_SCHMITT_ENABLE ; +break ; +case PIN_CONFIG_SLEW_RATE : + mask |=PAD_SLEW_RATE_MASK ; + value = (value & ~PAD_SLEW_RATE_MASK ) | + ((arg <<PAD_SLEW_RATE_POS ) &PAD_SLEW_RATE_MASK ); +break ; +case PIN_CONFIG_STARFIVE_STRONG_PULL_UP : +if (arg) { + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) | +PAD_BIAS_STRONG_PULL_UP ; + }else { + mask |=PAD_BIAS_STRONG_PULL_UP ; + value = value & ~PAD_BIAS_STRONG_PULL_UP ; + } +break ; +default : +return -ENOTSUPP ;
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 881
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+ } + } + +for (i =0 ; i < group->num_pins; i++) +starfive_padctl_rmw (sfp, group->pins[i], mask, value); + +return 0 ; +}
Linus any comments on this code (sorry if I missed your reply)? The idea behind above is to skip all settings from the same category and apply only the last one, e.g. if we have "bias set to X", ..., "bias disable", ..., "bias set to Y", the hardware will see only the last operation, i.e. "bias set to Y". I think it may not be the best approach (theoretically?) since the hardware definitely may behave differently on the other side in case of such series of the configurations (yes, I have seen some interesting implementations of the touchpad / touchscreen GPIOs that may be affected).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko <[email protected]> wrote:
(...)
Quoted 87 lines
Quoted 1 line · Emil Renner Berthing
...
Quoted 73 lines
Quoted 73 lines
drivers/pinctrl/pinctrl-starfive.c new file +73
-0+static int starfive_pinconf_group_set (struct pinctrl_dev *pctldev, +unsigned int gsel, +unsigned long *configs, +unsigned int num_configs) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +const struct group_desc *group; +u16 mask, value; +int i; + + group =pinctrl_generic_get_group (pctldev, gsel); +if (!group) +return -EINVAL ; + + mask =0 ; + value =0 ; +for (i =0 ; i < num_configs; i++) { +int param =pinconf_to_config_param (configs[i]); +u32 arg =pinconf_to_config_argument (configs[i]); + +switch (param) { +case PIN_CONFIG_BIAS_DISABLE : + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_DISABLE ; +break ; +case PIN_CONFIG_BIAS_PULL_DOWN : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_PULL_DOWN ; +break ; +case PIN_CONFIG_BIAS_PULL_UP : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = value & ~PAD_BIAS_MASK ; +break ; +case PIN_CONFIG_DRIVE_STRENGTH : + mask |=PAD_DRIVE_STRENGTH_MASK ; + value = (value & ~PAD_DRIVE_STRENGTH_MASK ) | +starfive_drive_strength_from_max_mA (arg); +break ; +case PIN_CONFIG_INPUT_ENABLE : + mask |=PAD_INPUT_ENABLE ; +if (arg) + value |=PAD_INPUT_ENABLE ; +else + value &= ~PAD_INPUT_ENABLE ; +break ; +case PIN_CONFIG_INPUT_SCHMITT_ENABLE : + mask |=PAD_INPUT_SCHMITT_ENABLE ; +if (arg) + value |=PAD_INPUT_SCHMITT_ENABLE ; +else + value &= ~PAD_INPUT_SCHMITT_ENABLE ; +break ; +case PIN_CONFIG_SLEW_RATE : + mask |=PAD_SLEW_RATE_MASK ; + value = (value & ~PAD_SLEW_RATE_MASK ) | + ((arg <<PAD_SLEW_RATE_POS ) &PAD_SLEW_RATE_MASK ); +break ; +case PIN_CONFIG_STARFIVE_STRONG_PULL_UP : +if (arg) { + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) | +PAD_BIAS_STRONG_PULL_UP ; + }else { + mask |=PAD_BIAS_STRONG_PULL_UP ; + value = value & ~PAD_BIAS_STRONG_PULL_UP ; + } +break ; +default : +return -ENOTSUPP ;Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 881
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+ } + } + +for (i =0 ; i < group->num_pins; i++) +starfive_padctl_rmw (sfp, group->pins[i], mask, value); + +return 0 ; +}Linus any comments on this code (sorry if I missed your reply)? The
Quoted 8 lines
idea behind above is to skip all settings from the same category and apply only the last one, e.g. if we have "bias set to X", ..., "bias disable", ..., "bias set to Y", the hardware will see only the last operation, i.e. "bias set to Y". I think it may not be the best approach (theoretically?) since the hardware definitely may behave differently on the other side in case of such series of the configurations (yes, I have seen some interesting implementations of the touchpad / touchscreen GPIOs that may be affected).
That sounds weird. I think we need to look at how other drivers deal with this.
To me it seems more natural that starfive_padctl_rmw(sfp, group->pins[i], mask, value); would get called at the end of each iteration of the for (i = 0; i < num_configs; i++) loop.
Yours,
Linus Walleij
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 100 lines · Linus Walleij
On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko <[email protected]> wrote:
(...)Quoted 87 lines
Quoted 1 line · Emil Renner Berthing
...
Quoted 73 lines
Quoted 73 lines
drivers/pinctrl/pinctrl-starfive.c new file +73
-0+static int starfive_pinconf_group_set (struct pinctrl_dev *pctldev, +unsigned int gsel, +unsigned long *configs, +unsigned int num_configs) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +const struct group_desc *group; +u16 mask, value; +int i; + + group =pinctrl_generic_get_group (pctldev, gsel); +if (!group) +return -EINVAL ; + + mask =0 ; + value =0 ; +for (i =0 ; i < num_configs; i++) { +int param =pinconf_to_config_param (configs[i]); +u32 arg =pinconf_to_config_argument (configs[i]); + +switch (param) { +case PIN_CONFIG_BIAS_DISABLE : + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_DISABLE ; +break ; +case PIN_CONFIG_BIAS_PULL_DOWN : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) |PAD_BIAS_PULL_DOWN ; +break ; +case PIN_CONFIG_BIAS_PULL_UP : +if (arg ==0 ) +return -ENOTSUPP ; + mask |=PAD_BIAS_MASK ; + value = value & ~PAD_BIAS_MASK ; +break ; +case PIN_CONFIG_DRIVE_STRENGTH : + mask |=PAD_DRIVE_STRENGTH_MASK ; + value = (value & ~PAD_DRIVE_STRENGTH_MASK ) | +starfive_drive_strength_from_max_mA (arg); +break ; +case PIN_CONFIG_INPUT_ENABLE : + mask |=PAD_INPUT_ENABLE ; +if (arg) + value |=PAD_INPUT_ENABLE ; +else + value &= ~PAD_INPUT_ENABLE ; +break ; +case PIN_CONFIG_INPUT_SCHMITT_ENABLE : + mask |=PAD_INPUT_SCHMITT_ENABLE ; +if (arg) + value |=PAD_INPUT_SCHMITT_ENABLE ; +else + value &= ~PAD_INPUT_SCHMITT_ENABLE ; +break ; +case PIN_CONFIG_SLEW_RATE : + mask |=PAD_SLEW_RATE_MASK ; + value = (value & ~PAD_SLEW_RATE_MASK ) | + ((arg <<PAD_SLEW_RATE_POS ) &PAD_SLEW_RATE_MASK ); +break ; +case PIN_CONFIG_STARFIVE_STRONG_PULL_UP : +if (arg) { + mask |=PAD_BIAS_MASK ; + value = (value & ~PAD_BIAS_MASK ) | +PAD_BIAS_STRONG_PULL_UP ; + }else { + mask |=PAD_BIAS_STRONG_PULL_UP ; + value = value & ~PAD_BIAS_STRONG_PULL_UP ; + } +break ; +default : +return -ENOTSUPP ;Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 881
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+ } + } + +for (i =0 ; i < group->num_pins; i++) +starfive_padctl_rmw (sfp, group->pins[i], mask, value); + +return 0 ; +}Linus any comments on this code (sorry if I missed your reply)? The
Quoted 8 lines
idea behind above is to skip all settings from the same category and apply only the last one, e.g. if we have "bias set to X", ..., "bias disable", ..., "bias set to Y", the hardware will see only the last operation, i.e. "bias set to Y". I think it may not be the best approach (theoretically?) since the hardware definitely may behave differently on the other side in case of such series of the configurations (yes, I have seen some interesting implementations of the touchpad / touchscreen GPIOs that may be affected).
Quoted 7 lines
That sounds weird. I think we need to look at how other drivers deal with this.
To me it seems more natural that
starfive_padctl_rmw(sfp, group->pins[i], mask, value); would get called at the end of each iteration of the for (i = 0; i <num_configs; i++) loop.
That would work, but when the loop is done the end result would be exactly the same. The only difference is that the above would rapidly "blink" the different states during the loop until it arrives at the result. This would certainly be different, but it can never be the intended behaviour and only a side-effect on how the pinctrl framework works. The order the different states are blinked depends entirely on how the pinctrl framework parses the device tree. I still think it would be more natural to cleanly go to the end result without this blinking.
/Emil
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 3 lines · Emil Renner Berthing
Quoted 2 lines · Linus Walleij
On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko <[email protected]> wrote:
...
Quoted 13 lines
Quoted 13 lines
Quoted 9 lines
Linus any comments on this code (sorry if I missed your reply)? The idea behind above is to skip all settings from the same category and apply only the last one, e.g. if we have "bias set to X", ..., "bias disable", ..., "bias set to Y", the hardware will see only the last operation, i.e. "bias set to Y". I think it may not be the best approach (theoretically?) since the hardware definitely may behave differently on the other side in case of such series of the configurations (yes, I have seen some interesting implementations of the touchpad / touchscreen GPIOs that may be affected).
That sounds weird. I think we need to look at how other drivers deal with this.
Quoted 7 lines
Quoted 4 lines
To me it seems more natural that
starfive_padctl_rmw(sfp, group->pins[i], mask, value); would get called at the end of each iteration of the for (i = 0; i <num_configs; i++) loop.That would work, but when the loop is done the end result would be exactly the same.
It seems we interpret the term "result" differently. The result when we talking about GPIOs is the series of pin state changes incl. configuration. This is how it should be recognized when programming hardware.
Quoted 5 lines
The only difference is that the above would rapidly
"blink" the different states during the loop until it arrives at the result. This would certainly be different, but it can never be the intended behaviour and only a side-effect on how the pinctrl framework works.
Is it? That's what I'm trying to get an answer to. If you may guarantee this (the keywords "intended behaviour" and "side effect"), I wouldn't object.
Quoted 4 lines
The order the different states are blinked depends entirely on how the pinctrl framework parses the device tree. I still think it would be more natural to cleanly go to the end result without this blinking.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 39 lines · Andy Shevchenko
Quoted 3 lines · Emil Renner Berthing
Quoted 2 lines · Linus Walleij
On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko <[email protected]> wrote:
...
Quoted 13 lines
Quoted 13 lines
Quoted 9 lines
Linus any comments on this code (sorry if I missed your reply)? The idea behind above is to skip all settings from the same category and apply only the last one, e.g. if we have "bias set to X", ..., "bias disable", ..., "bias set to Y", the hardware will see only the last operation, i.e. "bias set to Y". I think it may not be the best approach (theoretically?) since the hardware definitely may behave differently on the other side in case of such series of the configurations (yes, I have seen some interesting implementations of the touchpad / touchscreen GPIOs that may be affected).
That sounds weird. I think we need to look at how other drivers deal with this.
Quoted 7 lines
Quoted 4 lines
To me it seems more natural that
starfive_padctl_rmw(sfp, group->pins[i], mask, value); would get called at the end of each iteration of the for (i = 0; i <num_configs; i++) loop.That would work, but when the loop is done the end result would be exactly the same.
It seems we interpret the term "result" differently. The result when we talking about GPIOs is the series of pin state changes incl. configuration. This is how it should be recognized when programming hardware.
Quoted 5 lines
The only difference is that the above would rapidly
"blink" the different states during the loop until it arrives at the result. This would certainly be different, but it can never be the intended behaviour and only a side-effect on how the pinctrl framework works.
Quoted 8 lines
Is it? That's what I'm trying to get an answer to. If you may guarantee this (the keywords "intended behaviour" and "side effect"), I wouldn't object.
Quoted 4 lines
The order the different states are blinked depends entirely on how the pinctrl framework parses the device tree. I still think it would be more natural to cleanly go to the end result without this blinking.
Hmm.. but if going through the different states is what you want, then wouldn't you need the device tree to have an ordered list of the states rather than just a single node and also a way to tune how long time the different states are blinked?
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 11 lines · Emil Renner Berthing
Quoted 4 lines · Andy Shevchenko
Quoted 4 lines
The order the different states are blinked depends entirely on how the pinctrl framework parses the device tree. I still think it would be more natural to cleanly go to the end result without this blinking.
Hmm.. but if going through the different states is what you want, then wouldn't you need the device tree to have an ordered list of the states rather than just a single node and also a way to tune how long time the different states are blinked?
In a way you are correct that the DT is a functional language and it's a bit lite a style sheet or prolog or something in that the end reduction is what counts.
In this case, I would say something is weird if there are interim states, the yaml validation should not allow you to set the same thing back and forth in your DTS file.
Alas we are not perfect as in yaml validation isn't perfect either. I can't see what the problem is really, just write proper DTS files and there will not be any interim states, right? And if it is possible to write DTS files that have states and sequence requirements, these should be caught in validation. Should be.
Yours,
Linus Walleij
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 13 lines · Linus Walleij
Quoted 11 lines · Emil Renner Berthing
Quoted 4 lines · Andy Shevchenko
Quoted 4 lines
The order the different states are blinked depends entirely on how the pinctrl framework parses the device tree. I still think it would be more natural to cleanly go to the end result without this blinking.
Hmm.. but if going through the different states is what you want, then wouldn't you need the device tree to have an ordered list of the states rather than just a single node and also a way to tune how long time the different states are blinked?
Quoted 7 lines
In a way you are correct that the DT is a functional language and it's a bit lite a style sheet or prolog or something in that the end reduction is what counts.
In this case, I would say something is weird if there are interim states, the yaml validation should not allow you to set the same thing back and forth in your DTS file.
Yes, exactly.
Quoted 3 lines
Alas we are not perfect as in yaml validation isn't perfect either. I can't see what the problem is really, just write proper DTS files and there will not be any interim states, right?
No, I agree. I think it's only that Andy wasn't sure if these interim states might be meaningful/useful.
Quoted 6 lines
And if it is possible
to write DTS files that have states and sequence requirements, these should be caught in validation. Should be.Yours,
Linus Walleij
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Linus Walleij
On Tue, Nov 9, 2021 at 10:40 AM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 2 lines
No, I agree. I think it's only that Andy wasn't sure if these interim states might be meaningful/useful.
Exactly. Because HW could behave differently.
Quoted 3 lines
Quoted 3 lines
And if it is possible
to write DTS files that have states and sequence requirements, these should be caught in validation. Should be.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 10 lines · Andy Shevchenko
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Linus Walleij
On Tue, Nov 9, 2021 at 10:40 AM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 2 lines
No, I agree. I think it's only that Andy wasn't sure if these interim states might be meaningful/useful.
Exactly. Because HW could behave differently.
Right. But I think we've now established that what is described in the device tree is the state the pins should be in after the function has been called, eg. only the reduction matters, and any interim states would just be a byproduct of storing the state in the configs list.
Quoted 14 lines
Quoted 3 lines
Quoted 3 lines
And if it is possible
to write DTS files that have states and sequence requirements, these should be caught in validation. Should be.--
With Best Regards,
Andy Shevchenko_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
In reply to Andy ShevchenkoQuoted 26 lines · Andy Shevchenko
Quoted 15 lines · Emil Renner Berthing
Add a combined pinctrl and GPIO driver for the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC, which is said to feature only minor changes to these pinctrl/GPIO parts.
For each "GPIO" there are two registers for configuring the output and output enable signals which may come from other peripherals. Among these are two special signals that are constant 0 and constant 1 respectively. Controlling the GPIOs from software is done by choosing one of these signals. In other words the same registers are used for both pin muxing and controlling the GPIOs, which makes it easier to combine the pinctrl and GPIO driver in one.
I wrote the pinconf and pinmux parts, but the GPIO part of the code is based on the GPIO driver in the vendor tree written by Huan Feng with cleanups and fixes by Drew and me.
...
Quoted 1 line
+ depends on OF
So this descreases test coverage.
Linus, can we provide a necessary stub so we may drop this dependency?...
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 216
Quoted 5 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+static inline struct device *starfive_dev (const struct starfive_pinctrl *sfp) +{ +return sfp->gc.parent; +} +This seems useless helper. You may do what it's doing just in place. It will save 5 LOCs.
I don't mind removing it, I just think it's easier to read when we're explicit that all we want is a dev pointer, and we don't suddenly need to know the parent of the gpio chip in all the pinmux/pinconf callbacks.
Quoted 9 lines
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+static void starfive_pin_dbg_show (struct pinctrl_dev *pctldev, +struct seq_file *s, +unsigned int pin) +{ +struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +unsigned int gpio =starfive_pin_to_gpio (sfp, pin); +void __iomem *reg; +u32 dout, doen;
Quoted 4 lines
drivers/pinctrl/pinctrl-starfive.c line 463
Quoted 2 lines
drivers/pinctrl/pinctrl-starfive.c new file +2
-0+if (gpio >=NR_GPIOS ) +return ;Dead code?
No, this function is called for all 206 configurable pins, but only 64 of them are gpio pins. Which ones depend on the signal group.
Quoted 22 lines
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+ reg = sfp->base +GPON_DOUT_CFG +8 * gpio; + dout =readl_relaxed (reg +0x000 ); + doen =readl_relaxed (reg +0x004 ); + +seq_printf (s,"dout=%lu%s doen=%lu%s" , + dout &GENMASK (7 ,0 ), (dout &BIT (31 )) ?"r" :"" , + doen &GENMASK (7 ,0 ), (doen &BIT (31 )) ?"r" :"" ); +}...
drivers/pinctrl/pinctrl-starfive.c line 489
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c new file +8
-0+struct starfive_pinctrl *sfp =pinctrl_dev_get_drvdata (pctldev); +struct device *dev =starfive_dev (sfp); +const char **pgnames; +struct pinctrl_map *map; +struct device_node *child; +const char *grpname; +int *pins; +u32 *pinmux;Reversed xmas tree order?
Quoted 9 lines
drivers/pinctrl/pinctrl-starfive.c line 933
Quoted 7 lines
drivers/pinctrl/pinctrl-starfive.c new file +7
-0+static int starfive_gpio_get_direction (struct gpio_chip *gc,unsigned int gpio) +{ + struct starfive_pinctrl *sfp = container_of(gc, struct starfive_pinctrl, gc); +void __iomem *doen = sfp->base +GPON_DOEN_CFG +8 * gpio; + +/* return GPIO_LINE_DIRECTION_OUT (0) only if doen == GPO_ENABLE (0) */ +return readl_relaxed (doen) !=GPO_ENABLE ;I believe the idea was to return the predefined values for the direction.
You mean this?
return readl_relaxed(doen) == GPO_ENABLE ? GPIO_LINE_DIRECTION_OUT :GPIO_LINE_DIRECTION_IN;
Quoted 48 lines
Quoted 1 line
+}
...
drivers/pinctrl/pinctrl-starfive.c line 1126
Quoted 13 lines
drivers/pinctrl/pinctrl-starfive.c new file +13
-0+static int starfive_irq_set_type (struct irq_data *d,unsigned int trigger) +{ +struct starfive_pinctrl *sfp =starfive_from_irq_data (d); +irq_hw_number_t gpio =irqd_to_hwirq (d); +void __iomem *base = sfp->base +4 * (gpio /32 ); +u32 mask =BIT (gpio %32 ); +u32 irq_type, edge_both, polarity; +unsigned long flags; + +if (trigger &IRQ_TYPE_EDGE_BOTH ) +irq_set_handler_locked (d, handle_edge_irq); +else if (trigger &IRQ_TYPE_LEVEL_MASK ) +irq_set_handler_locked (d, handle_level_irq);Usually we don't assign this twice, so it should be after the switch.
Quoted 27 lines
drivers/pinctrl/pinctrl-starfive.c new file +27
-0+switch (trigger) { +case IRQ_TYPE_EDGE_RISING : + irq_type = mask;/* 1: edge triggered */ + edge_both =0 ;/* 0: single edge */ + polarity = mask;/* 1: rising edge */ +break ; +case IRQ_TYPE_EDGE_FALLING : + irq_type = mask;/* 1: edge triggered */ + edge_both =0 ;/* 0: single edge */ + polarity =0 ;/* 0: falling edge */ +break ; +case IRQ_TYPE_EDGE_BOTH : + irq_type = mask;/* 1: edge triggered */ + edge_both = mask;/* 1: both edges */ + polarity =0 ;/* 0: ignored */ +break ; +case IRQ_TYPE_LEVEL_HIGH : + irq_type =0 ;/* 0: level triggered */ + edge_both =0 ;/* 0: ignored */ + polarity = mask;/* 1: high level */ +break ; +case IRQ_TYPE_LEVEL_LOW : + irq_type =0 ;/* 0: level triggered */ + edge_both =0 ;/* 0: ignored */ + polarity =0 ;/* 0: low level */ +break ; +default :
Quoted 3 lines
Quoted 1 line
+
irq_set_handler_locked(d, handle_bad_irq);Why? You have it already in ->probe(), what's the point?
So last time you asked about this, I explained a situation where userspace first grabs a GPIO, set the interrupt to edge triggered, and then later loads a driver that requests an unsupported IRQ type. Then I'd like to set the handler back to handle_bad_irq so we don't get weird interrupts, but maybe now you know a reason why that doesn't matter or can't happen?
Quoted 16 lines
Quoted 13 lines
drivers/pinctrl/pinctrl-starfive.c new file +13
-0+return -EINVAL ; + } + +raw_spin_lock_irqsave (&sfp->lock, flags); + irq_type |=readl_relaxed (base +GPIOIS ) & ~mask; +writel_relaxed (irq_type, base +GPIOIS ); + edge_both |=readl_relaxed (base +GPIOIBE ) & ~mask; +writel_relaxed (edge_both, base +GPIOIBE ); + polarity |=readl_relaxed (base +GPIOIEV ) & ~mask; +writel_relaxed (polarity, base +GPIOIEV ); +raw_spin_unlock_irqrestore (&sfp->lock, flags); +return 0 ; +}...
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 1265
Quoted 6 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+ ret =reset_control_deassert (rst); +if (ret) +return dev_err_probe (dev, ret,"could not deassert resetd\n" ); + ret =devm_pinctrl_register_and_init (dev, &starfive_desc, sfp, &sfp->pctl); +if (ret)I don't see who will assert reset here.
No, so originally this driver would first assert and then deassert reset. I decided against that because in all likelyhood earlier boot stages would have set pinmux up for a serial port, and we don't want to interrupt the serial debug output. The only reason I make sure the reset line is deasserted is in case someone makes a really minimal bootloader that just does the absolute minimal to load a Linux kernel and doesn't even log any anything.
By the same token we also don't want to assert reset on error in case it resets pin muxing for the the serial line that was supposed to log the error.
Quoted 42 lines
Quoted 2 lines
+ return
dev_err_probe(dev, ret, "could not register pinctrl driver\n");...
Quoted 7 lines
drivers/pinctrl/pinctrl-starfive.c new file +7
-0+switch (value) { +case 0 : + sfp->gpios.pin_base =PAD_INVALID_GPIO ; + goto done; +case 1 : + sfp->gpios.pin_base =PAD_GPIO (0 ); +break ;drivers/pinctrl/pinctrl-starfive.c line 1291
Quoted 10 lines
drivers/pinctrl/pinctrl-starfive.c new file +10
-0+case 2 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (72 ); +break ; +case 3 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (70 ); +break ; +case 4 :case 5 :case 6 : + sfp->gpios.pin_base =PAD_FUNC_SHARE (0 ); +break ; +default :Ditto.
Quoted 2 lines
(quoted diff) +2
-0+ return dev_err_probe(dev, -EINVAL, "invalid signal group %u\n", value); + }...
drivers/pinctrl/pinctrl-starfive.c line 1329
Quoted 2 lines
drivers/pinctrl/pinctrl-starfive.c new file +2
-0+ ret =devm_gpiochip_add_data (dev, &sfp->gc, sfp); +if (ret)Ditto.
drivers/pinctrl/pinctrl-starfive.c line 1333
Quoted 4 lines
drivers/pinctrl/pinctrl-starfive.c new file +4
-0+return dev_err_probe (dev, ret,"could not register gpiochip\n" ); + +done: +return pinctrl_enable (sfp->pctl);Ditto.
Quoted 2 lines
And better to use label name like following
out_pinctrl_enable:
Good idea.
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 3 lines · Emil Renner Berthing
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 9 lines
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 216
Quoted 5 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+static inline struct device *starfive_dev (const struct starfive_pinctrl *sfp) +{ +return sfp->gc.parent; +} +This seems useless helper. You may do what it's doing just in place. It will save 5 LOCs.
Quoted 4 lines
I don't mind removing it, I just think it's easier to read when we're explicit that all we want is a dev pointer, and we don't suddenly need to know the parent of the gpio chip in all the pinmux/pinconf callbacks.
I don't really see the gain of it.
...
Quoted 10 lines
Quoted 9 lines
drivers/pinctrl/pinctrl-starfive.c line 933
Quoted 7 lines
drivers/pinctrl/pinctrl-starfive.c new file +7
-0+static int starfive_gpio_get_direction (struct gpio_chip *gc,unsigned int gpio) +{ + struct starfive_pinctrl *sfp = container_of(gc, struct starfive_pinctrl, gc); +void __iomem *doen = sfp->base +GPON_DOEN_CFG +8 * gpio; + +/* return GPIO_LINE_DIRECTION_OUT (0) only if doen == GPO_ENABLE (0) */ +return readl_relaxed (doen) !=GPO_ENABLE ;I believe the idea was to return the predefined values for the direction.
Quoted 3 lines
You mean this?
returnreadl_relaxed(doen)==GPO_ENABLE?GPIO_LINE_DIRECTION_OUT:GPIO_LINE_DIRECTION_IN;
For example, or with if (...) return _OUT; return _IN;'
Quoted 1 line
Quoted 1 line
Quoted 1 line
+}
...
Quoted 11 lines
Quoted 11 lines
drivers/pinctrl/pinctrl-starfive.c line 1126
Quoted 4 lines
drivers/pinctrl/pinctrl-starfive.c new file +4
-0+if (trigger &IRQ_TYPE_EDGE_BOTH ) +irq_set_handler_locked (d, handle_edge_irq); +else if (trigger &IRQ_TYPE_LEVEL_MASK ) +irq_set_handler_locked (d, handle_level_irq);Usually we don't assign this twice, so it should be after the switch.
Quoted 3 lines
drivers/pinctrl/pinctrl-starfive.c new file +2
-0+switch (trigger) { +default :
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
+
irq_set_handler_locked(d, handle_bad_irq);Why? You have it already in ->probe(), what's the point?
So last time you asked about this, I explained a situation where userspace first grabs a GPIO, set the interrupt to edge triggered, and then later loads a driver that requests an unsupported IRQ type.
I didn't get this scenario. Is it real?
Quoted 4 lines
Then
I'd like to set the handler back tohandle_bad_irqso we don't get weird interrupts, but maybe now you know a reason why that doesn't matter or can't happen?
In ->probe() you set _default_ handler to bad(), what do you mean by 'set the handler back to bad()'? How is it otherwise if you free an interrupt?
So, please elaborate with call traces what the scenario / use case you are talking about. If it's true what you are saying, we have a situation (plenty of GPIO drivers don't do what you are suggesting here).
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
(quoted diff) +2
-0+ return -EINVAL; + }
...
Quoted 17 lines
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 1265
Quoted 6 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+ ret =reset_control_deassert (rst); +if (ret) +return dev_err_probe (dev, ret,"could not deassert resetd\n" ); + ret =devm_pinctrl_register_and_init (dev, &starfive_desc, sfp, &sfp->pctl); +if (ret)I don't see who will assert reset here.
No, so originally this driver would first assert and then deassert reset. I decided against that because in all likelyhood earlier boot stages would have set pinmux up for a serial port, and we don't want to interrupt the serial debug output. The only reason I make sure the reset line is deasserted is in case someone makes a really minimal bootloader that just does the absolute minimal to load a Linux kernel and doesn't even log any anything.
Quoted 3 lines
By the same token we also don't want to assert reset on error in case it resets pin muxing for the the serial line that was supposed to log the error.
Perhaps comment in the code explaining this?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerRe: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 10 lines · Andy Shevchenko
Quoted 9 lines · Emil Renner Berthing
Quoted 8 lines · Andy Shevchenko
Quoted 5 lines · Emil Renner Berthing
drivers/pinctrl/pinctrl-starfive.c new file +3
-0+switch (trigger) { +default : +irq_set_handler_locked (d, handle_bad_irq);
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Why? You have it already in ->probe(), what's the point?
So last time you asked about this, I explained a situation where userspace first grabs a GPIO, set the interrupt to edge triggered, and then later loads a driver that requests an unsupported IRQ type.
I didn't get this scenario. Is it real?
No, it's totally made up, but I mean we even have tools like fuzzing to help us find bugs that would never happen in real use cases.
Quoted 8 lines
Quoted 4 lines
Then
I'd like to set the handler back tohandle_bad_irqso we don't get weird interrupts, but maybe now you know a reason why that doesn't matter or can't happen?In ->probe() you set _default_ handler to
bad(), what do you mean by 'set the handler back tobad()'? How is it otherwise if you free an interrupt?
It might not be, but when not sure I thought it better to error on the safe side.
Quoted 27 lines
So, please elaborate with call traces what the scenario / use case you are talking about. If it's true what you are saying, we have a situation (plenty of GPIO drivers don't do what you are suggesting here).
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
(quoted diff) +2
-0+ return -EINVAL; + }...
Quoted 17 lines
Quoted 8 lines
drivers/pinctrl/pinctrl-starfive.c line 1265
Quoted 6 lines
drivers/pinctrl/pinctrl-starfive.c new file +5
-0+ ret =reset_control_deassert (rst); +if (ret) +return dev_err_probe (dev, ret,"could not deassert resetd\n" ); + ret =devm_pinctrl_register_and_init (dev, &starfive_desc, sfp, &sfp->pctl); +if (ret)I don't see who will assert reset here.
No, so originally this driver would first assert and then deassert reset. I decided against that because in all likelyhood earlier boot stages would have set pinmux up for a serial port, and we don't want to interrupt the serial debug output. The only reason I make sure the reset line is deasserted is in case someone makes a really minimal bootloader that just does the absolute minimal to load a Linux kernel and doesn't even log any anything.
Quoted 5 lines
Quoted 3 lines
By the same token we also don't want to assert reset on error in case it resets pin muxing for the the serial line that was supposed to log the error.
Perhaps comment in the code explaining this?
Sure.
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 4 lines · Emil Renner Berthing
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Emil Renner Berthing
Quoted 1 line · Andy Shevchenko
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <[email protected]> wrote:
...
Quoted 18 lines
Quoted 9 lines
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
+
irq_set_handler_locked(d, handle_bad_irq);Why? You have it already in ->probe(), what's the point?
So last time you asked about this, I explained a situation where userspace first grabs a GPIO, set the interrupt to edge triggered, and then later loads a driver that requests an unsupported IRQ type.
I didn't get this scenario. Is it real?
No, it's totally made up, but I mean we even have tools like fuzzing to help us find bugs that would never happen in real use cases.
Quoted 5 lines
Quoted 4 lines
Then
I'd like to set the handler back tohandle_bad_irqso we don't get weird interrupts, but maybe now you know a reason why that doesn't matter or can't happen?
Quoted 6 lines
Quoted 3 lines
In ->probe() you set _default_ handler to
bad(), what do you mean by 'set the handler back tobad()'? How is it otherwise if you free an interrupt?It might not be, but when not sure I thought it better to error on the safe side.
With a dead code?
I do not believe there is an issue since. like I said, there are plenty drivers that don't do what you are suggesting here --> 99.99% you added a dead code.
Quoted 4 lines
Quoted 4 lines
So, please elaborate with call traces what the scenario / use case you are talking about. If it's true what you are saying, we have a situation (plenty of GPIO drivers don't do what you are suggesting here).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
In reply to Andy ShevchenkoQuoted 4 lines · Andy Shevchenko
Quoted 1 line
+ depends on OF
So this descreases test coverage.
Linus, can we provide a necessary stub so we may drop this dependency?
Hm it further selects OF_GPIO which depends on OF so I don't know how that would work.
But does it decrease compile coverage a lot, even x86 has optional OF support so I imagine it appears in x86 allyesconfig I suppose? Or am I wrong?
Yours,
Linus Walleij
Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
Quoted 8 lines · Linus Walleij
Quoted 4 lines · Andy Shevchenko
Quoted 1 line
+ depends on OF
So this descreases test coverage.
Linus, can we provide a necessary stub so we may drop this dependency?
Quoted 6 lines
Hm it further selects
OF_GPIOwhich depends on OF so I don't know how that would work.But does it decrease compile coverage a lot, even x86 has optional OF support so I imagine it appears in x86 allyesconfig I suppose? Or am I wrong?
I believe so. At least in my environment I have OF enabled (I haven't looked into what was the change to the config, though).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Emil Renner Berthing
maintainerPATCH v3 13/16 dt-bindings: serial: snps-dw-apb-uart: Add JH7100 uarts
In reply to Emil Renner BerthingAdd compatibles for the StarFive JH7100 uarts.
--- .../devicetree/bindings/serial/snps-dw-apb-uart.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml +5 -0
@@ -40,6 +40,11 @@ properties: - brcm,bcm11351-dw-apb-uart - brcm,bcm21664-dw-apb-uart - const: snps,dw-apb-uart + - items: + - enum: + - starfive,jh7100-hsuart + - starfive,jh7100-uart + - const: snps,dw-apb-uart - const: snps,dw-apb-uart reg:
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 14/16 serial: 8250_dw: Add StarFive JH7100 quirk
In reply to Emil Renner BerthingOn the StarFive JH7100 RISC-V SoC the UART core clocks can't be set to exactly 16 * 115200Hz and many other common bitrates. Trying this will only result in a higher input clock, but low enough that the UART's internal divisor can't come close enough to the baud rate target. So rather than try to set the input clock it's better to skip the clk_set_rate call and rely solely on the UART's internal divisor.
--- drivers/tty/serial/8250/8250_dw.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/tty/serial/8250/8250_dw.c +3 -0
@@ -414,6 +414,8 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)if (of_device_is_compatible (np,"marvell,armada-38x-uart" )) p->serial_out = dw8250_serial_out38x; +if (of_device_is_compatible (np,"starfive,jh7100-uart" )) + p->set_termios = dw8250_do_set_termios; }else if (acpi_dev_present ("APMC0D08" ,NULL ,-1 )) { p->iotype =UPIO_MEM32 ;
@@ -696,6 +698,7 @@ static const struct of_device_id dw8250_of_match[] = { { .compatible = "cavium,octeon-3860-uart" }, { .compatible = "marvell,armada-38x-uart" }, { .compatible = "renesas,rzn1-uart" }, + { .compatible = "starfive,jh7100-uart" }, {/* Sentinel */ } };MODULE_DEVICE_TABLE (of, dw8250_of_match);
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 14/16] serial: 8250_dw: Add StarFive JH7100 quirk
Quoted 7 lines · Emil Renner Berthing
On the StarFive JH7100 RISC-V SoC the UART core clocks can't be set to exactly 16 * 115200Hz and many other common bitrates. Trying this will only result in a higher input clock, but low enough that the UART's internal divisor can't come close enough to the baud rate target. So rather than try to set the input clock it's better to skip the
clk_set_ratecall and rely solely on the UART's internal divisor.
Bingo!
Quoted 29 lines
Signed-off-by Emil Renner Berthing <[email protected]>--- drivers/tty/serial/8250/8250_dw.c | 3 +++ 1 file changed, 3 insertions(+)drivers/tty/serial/8250/8250_dw.c +3
-1@@ -414,6 +414,8 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)if (of_device_is_compatible (np,"marvell,armada-38x-uart" )) p->serial_out = dw8250_serial_out38x; +if (of_device_is_compatible (np,"starfive,jh7100-uart" )) + p->set_termios = dw8250_do_set_termios; }else if (acpi_dev_present ("APMC0D08" ,NULL ,-1 )) { p->iotype =UPIO_MEM32 ;@@ -696,6 +698,7 @@ static const struct of_device_id dw8250_of_match[] = { { .compatible = "cavium,octeon-3860-uart" }, { .compatible = "marvell,armada-38x-uart" }, { .compatible = "renesas,rzn1-uart" }, + { .compatible = "starfive,jh7100-uart" }, {/* Sentinel */ } };MODULE_DEVICE_TABLE (of, dw8250_of_match); --2.33.1
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 14/16] serial: 8250_dw: Add StarFive JH7100 quirk
In reply to Emil Renner BerthingHi Esmil,
Quoted 8 lines · Emil Renner Berthing
On the StarFive JH7100 RISC-V SoC the UART core clocks can't be set to exactly 16 * 115200Hz and many other common bitrates. Trying this will only result in a higher input clock, but low enough that the UART's internal divisor can't come close enough to the baud rate target. So rather than try to set the input clock it's better to skip the
clk_set_ratecall and rely solely on the UART's internal divisor.Signed-off-by Emil Renner Berthing <[email protected]>
Thanks for your patch!
drivers/tty/serial/8250/8250_dw.c static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) · line 418
Quoted 8 lines
drivers/tty/serial/8250/8250_dw.c +2
-0@@ -414,6 +414,8 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)if (of_device_is_compatible (np,"marvell,armada-38x-uart" )) p->serial_out = dw8250_serial_out38x; +if (of_device_is_compatible (np,"starfive,jh7100-uart" )) + p->set_termios = dw8250_do_set_termios;
BTW, it would be great for a follow-up patch to get rid of all these of_device_is_compatible() checks, and start using dw8250_of_match[...].data instead.
drivers/tty/serial/8250/8250_dw.c static const struct of_device_id dw8250_of_match[] = { · line 703
Quoted 10 lines
drivers/tty/serial/8250/8250_dw.c +1
-0}else if (acpi_dev_present ("APMC0D08" ,NULL ,-1 )) { p->iotype =UPIO_MEM32 ;@@ -696,6 +698,7 @@ static const struct of_device_id dw8250_of_match[] = { { .compatible = "cavium,octeon-3860-uart" }, { .compatible = "marvell,armada-38x-uart" }, { .compatible = "renesas,rzn1-uart" }, + { .compatible = "starfive,jh7100-uart" }, {/* Sentinel */ } };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Emil Renner Berthing
maintainerPATCH v3 15/16 RISC-V: Add initial StarFive JH7100 device tree
In reply to Emil Renner BerthingAdd initial device tree for the JH7100 RISC-V SoC by StarFive Ltd. This is a test chip for their upcoming JH7110 SoC.
The CPU and cache data is based on the device tree in the vendor u-boot port.
--- arch/riscv/boot/dts/starfive/jh7100.dtsi | 230 +++++++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 arch/riscv/boot/dts/starfive/jh7100.dtsi
arch/riscv/boot/dts/starfive/jh7100.dtsi new file +230 -0
@@ -0,0 +1,230 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/* + * Copyright (C) 2021 StarFive Technology Co., Ltd. + * Copyright (C) 2021 Emil Renner Berthing <[email protected]> + */ + +/dts-v1/; +#include <dt-bindings/clock/starfive-jh7100.h> +#include <dt-bindings/reset/starfive-jh7100.h> + +/ { + compatible = "starfive,jh7100"; + #address-cells = <2>; + #size-cells = <2>; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + compatible = "sifive,u74-mc", "riscv"; + reg = <0>; + d-cache-block-size = <64>; + d-cache-sets = <64>; + d-cache-size = <32768>; + d-tlb-sets = <1>; + d-tlb-size = <32>; + device_type = "cpu"; + i-cache-block-size = <64>; + i-cache-sets = <64>; + i-cache-size = <32768>; + i-tlb-sets = <1>; + i-tlb-size = <32>; + mmu-type = "riscv,sv39"; + riscv,isa = "rv64imafdc"; + tlb-split; + + cpu0_intc: interrupt-controller { + compatible = "riscv,cpu-intc"; + interrupt-controller; + #interrupt-cells = <1>; + }; + }; + + cpu@1 { + compatible = "sifive,u74-mc", "riscv"; + reg = <1>; + d-cache-block-size = <64>; + d-cache-sets = <64>; + d-cache-size = <32768>; + d-tlb-sets = <1>; + d-tlb-size = <32>; + device_type = "cpu"; + i-cache-block-size = <64>; + i-cache-sets = <64>; + i-cache-size = <32768>; + i-tlb-sets = <1>; + i-tlb-size = <32>; + mmu-type = "riscv,sv39"; + riscv,isa = "rv64imafdc"; + tlb-split; + + cpu1_intc: interrupt-controller { + compatible = "riscv,cpu-intc"; + interrupt-controller; + #interrupt-cells = <1>; + }; + }; + }; + + osc_sys: osc_sys { + compatible = "fixed-clock"; + #clock-cells = <0>; + /* This value must be overridden by the board */ + clock-frequency = <0>; + }; + + osc_aud: osc_aud { + compatible = "fixed-clock"; + #clock-cells = <0>; + /* This value must be overridden by the board */ + clock-frequency = <0>; + }; + + gmac_rmii_ref: gmac_rmii_ref { + compatible = "fixed-clock"; + #clock-cells = <0>; + /* Should be overridden by the board when needed */ + clock-frequency = <0>; + }; + + gmac_gr_mii_rxclk: gmac_gr_mii_rxclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + /* Should be overridden by the board when needed */ + clock-frequency = <0>; + }; + + soc { + compatible = "simple-bus"; + interrupt-parent = <&plic>; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + clint: clint@2000000 { + compatible = "starfive,jh7100-clint", "sifive,clint0"; + reg = <0x0 0x2000000 0x0 0x10000>; + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 + &cpu1_intc 3 &cpu1_intc 7>; + }; + + plic: interrupt-controller@c000000 { + compatible = "starfive,jh7100-plic", "sifive,plic-1.0.0"; + reg = <0x0 0xc000000 0x0 0x4000000>; + interrupts-extended = <&cpu0_intc 11 &cpu0_intc 9 + &cpu1_intc 11 &cpu1_intc 9>; + interrupt-controller; + #address-cells = <0>; + #interrupt-cells = <1>; + riscv,ndev = <127>; + }; + + clkgen: clock-controller@11800000 { + compatible = "starfive,jh7100-clkgen"; + reg = <0x0 0x11800000 0x0 0x10000>; + clocks = <&osc_sys>, <&osc_aud>, <&gmac_rmii_ref>, <&gmac_gr_mii_rxclk>; + clock-names = "osc_sys", "osc_aud", "gmac_rmii_ref", "gmac_gr_mii_rxclk"; + #clock-cells = <1>; + }; + + rstgen: reset-controller@11840000 { + compatible = "starfive,jh7100-reset"; + reg = <0x0 0x11840000 0x0 0x10000>; + #reset-cells = <1>; + }; + + i2c0: i2c@118b0000 { + compatible = "snps,designware-i2c"; + reg = <0x0 0x118b0000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_I2C0_CORE>, + <&clkgen JH7100_CLK_I2C0_APB>; + clock-names = "ref", "pclk"; + resets = <&rstgen JH7100_RSTN_I2C0_APB>; + interrupts = <96>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + + i2c1: i2c@118c0000 { + compatible = "snps,designware-i2c"; + reg = <0x0 0x118c0000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_I2C1_CORE>, + <&clkgen JH7100_CLK_I2C1_APB>; + clock-names = "ref", "pclk"; + resets = <&rstgen JH7100_RSTN_I2C1_APB>; + interrupts = <97>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + + gpio: pinctrl@11910000 { + compatible = "starfive,jh7100-pinctrl"; + reg = <0x0 0x11910000 0x0 0x10000>, + <0x0 0x11858000 0x0 0x1000>; + reg-names = "gpio", "padctl"; + clocks = <&clkgen JH7100_CLK_GPIO_APB>; + resets = <&rstgen JH7100_RSTN_GPIO_APB>; + interrupts = <32>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + + uart2: serial@12430000 { + compatible = "starfive,jh7100-uart", "snps,dw-apb-uart"; + reg = <0x0 0x12430000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_UART2_CORE>, + <&clkgen JH7100_CLK_UART2_APB>; + clock-names = "baudclk", "apb_pclk"; + resets = <&rstgen JH7100_RSTN_UART2_APB>; + interrupts = <72>; + reg-io-width = <4>; + reg-shift = <2>; + status = "disabled"; + }; + + uart3: serial@12440000 { + compatible = "starfive,jh7100-uart", "snps,dw-apb-uart"; + reg = <0x0 0x12440000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_UART3_CORE>, + <&clkgen JH7100_CLK_UART3_APB>; + clock-names = "baudclk", "apb_pclk"; + resets = <&rstgen JH7100_RSTN_UART3_APB>; + interrupts = <73>; + reg-io-width = <4>; + reg-shift = <2>; + status = "disabled"; + }; + + i2c2: i2c@12450000 { + compatible = "snps,designware-i2c"; + reg = <0x0 0x12450000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_I2C2_CORE>, + <&clkgen JH7100_CLK_I2C2_APB>; + clock-names = "ref", "pclk"; + resets = <&rstgen JH7100_RSTN_I2C2_APB>; + interrupts = <74>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + + i2c3: i2c@12460000 { + compatible = "snps,designware-i2c"; + reg = <0x0 0x12460000 0x0 0x10000>; + clocks = <&clkgen JH7100_CLK_I2C3_CORE>, + <&clkgen JH7100_CLK_I2C3_APB>; + clock-names = "ref", "pclk"; + resets = <&rstgen JH7100_RSTN_I2C3_APB>; + interrupts = <75>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + }; +};
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Emil Renner Berthing
maintainerPATCH v3 16/16 RISC-V: Add BeagleV Starlight Beta device tree
In reply to Emil Renner BerthingAdd initial device tree for the BeagleV Starlight Beta board. About 300 of these boards were sent out as part of a now cancelled BeagleBoard.org project.
I2C timing data is based on the device tree in the vendor u-boot port. Heartbeat LED added by Geert.
--- arch/riscv/boot/dts/Makefile | 1 + arch/riscv/boot/dts/starfive/Makefile | 2 + .../dts/starfive/jh7100-beaglev-starlight.dts | 164 ++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 arch/riscv/boot/dts/starfive/Makefile create mode 100644 arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts
arch/riscv/boot/dts/Makefile +1 -0
@@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 subdir-y += sifive +subdir-y += starfive subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan subdir-y += microchip
arch/riscv/boot/dts/starfive/Makefile new file +2 -0
@@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 +dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts new file +164 -0
164 lines not on this page · raw message
Signature · 7 lines
2.33.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv