Skip to content

Commit

Permalink
pinctrl: starfive: Add pinctrl driver for the JH7100 SoC
Browse files Browse the repository at this point in the history 
This adds a combined pinctrl and gpio driver for the StarFive Ltd.
JH7100 RISC-V SoC [1].

For each "GPIO" there are two registers for configuring the output and
output enable signals, which may come from other peripherals, and
controlling the GPIOs from software amounts to setting these signals
to constant 0 or constant 1. In other words the same registers are used
for both pinmuxing and controlling the GPIOs, which makes it easier to
combine the pinctrl and gpio driver in one.

The gpio code is adapted from the gpio driver in the vendor tree written
by Huan Feng with cleanups and fixes by Drew and me.

[1] https://github.com/starfive-tech/beaglev_doc

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Huan Feng <huan.feng@starfivetech.com>
Signed-off-by: Drew Fustini <drew@beagleboard.org>
  • Loading branch information
@esmil
esmil committed Aug 2, 2021
1 parent 3096979 commit dd9648b
Show file tree
Hide file tree
Showing 5 changed files with 1,700 additions and 0 deletions.
7 changes: 7 additions & 0 deletions MAINTAINERS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17660,6 +17660,13 @@ M: Ion Badulescu <ionut@badula.org>
S: Odd Fixes
F: drivers/net/ethernet/adaptec/starfire*

STARFIVE JH7100 PINCTRL DRIVER
M: Emil Renner Berthing <kernel@esmil.dk>
L: linux-gpio@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
F: drivers/pinctrl/pinctrl-starfive.c

STATIC BRANCH/CALL
M: Peter Zijlstra <peterz@infradead.org>
M: Josh Poimboeuf <jpoimboe@redhat.com>
Expand Down
17 changes: 17 additions & 0 deletions drivers/pinctrl/Kconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,23 @@ config PINCTRL_ST
select PINCONF
select GPIOLIB_IRQCHIP

config PINCTRL_STARFIVE
bool "Pinctrl and GPIO driver for the StarFive JH7100 SoC"
depends on OF
depends on RISCV || COMPILE_TEST
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 Ltd. JH7100
RISC-V 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
Expand Down
1 change: 1 addition & 0 deletions drivers/pinctrl/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

11 comments on commit dd9648b

@andreas-schwab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ 8.577990] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 8.586807] Oops [#1]
[ 8.589074] Modules linked in: dw_mmc_pltfm(+) dw_mmc pwrseq_simple mmc_core gpio_tps65086 tps65086_regulator dw_axi_dmac_platform spi_dw_mmio spi_dw pwm_sifive_ptc tps65086 sfctemp mfd_core regmap_i2c virt_dma sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 8.613036] CPU: 0 PID: 7 Comm: kworker/u4:0 Tainted: G W 5.14.0-rc4-37-default #1 openSUSE Tumbleweed (unreleased) 215eb83313a070334a25c6635301c404012b7fa7
[ 8.628289] Hardware name: BeagleV Starlight Beta (DT)
[ 8.628304] Workqueue: events_unbound async_run_entry_fn
[ 8.638728] epc : strcmp+0x12/0x36
[ 8.642130] ra : pinmux_func_name_to_selector+0x46/0x62
[ 8.647436] epc : ffffffff8038fdc0 ra : ffffffff8039e106 sp : ffffffd00403bb30
[ 8.654629] gp : ffffffff8176e2e0 tp : ffffffe07fed9b80 t0 : 0000000000000040
[ 8.661823] t1 : 0000000000000000 t2 : ffffffff80c038c4 s0 : ffffffd00403bb40
[ 8.669015] s1 : 0000000000000007 a0 : ffffffe1ff1cadd8 a1 : 0000000000000000
[ 8.676206] a2 : 0000000000000000 a3 : 0000000000000000 a4 : ffffffe0802ff4e0
[ 8.683398] a5 : 0000000000000073 a6 : ffffffe0802ff488 a7 : ffffffe0802ff4e8
[ 8.690590] s2 : ffffffe07ff66780 s3 : 0000000000000008 s4 : ffffffe1ff1cadd8
[ 8.697785] s5 : ffffffff80c8fe08 s6 : ffffffe084c546c0 s7 : ffffffe084c54280
[ 8.704978] s8 : ffffffe0830665a0 s9 : ffffffe0808468c0 s10: ffffffe07ffa3b80
[ 8.712173] s11: 0000000000000001 t3 : 0000000000000002 t4 : 0000000000000402
[ 8.719367] t5 : ffffffe07ff667c0 t6 : ffffffe07ff667c8
[ 8.724658] status: 0000000200000120 badaddr: 0000000000000000 cause: 000000000000000d
[ 8.732544] [] strcmp+0x12/0x36
[ 8.737413] ---[ end trace 902ebac3a1a7b3ee ]---

@pdp7
Copy link
Collaborator

@pdp7 pdp7 commented on dd9648b Aug 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreas-schwab @esmil it looks like it was the strcmp in the while loop:
https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.c#L331

static int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
					const char *function)
{
	const struct pinmux_ops *ops = pctldev->desc->pmxops;
	unsigned nfuncs = ops->get_functions_count(pctldev);
	unsigned selector = 0;

	/* See if this pctldev has this function */
	while (selector < nfuncs) {
		const char *fname = ops->get_function_name(pctldev, selector);

		if (!strcmp(function, fname))
			return selector;

		selector++;
	}

	return -EINVAL;
}

It's hard to say though if it is the function (argument to the function we are in) or fname (from ops->get_function_name()) was NULL.

@andreas-schwab are you compiling from source? If so, could we toss in a print like:

pr_err("DEBUG %s function=%px fname=%px\n", __func__, function, fname);

@esmil
Copy link
Collaborator Author

@esmil esmil commented on dd9648b Aug 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could also try this patch and see if it's the generic pinmux functions that needs to be serialized: http://sprunge.us/JygyFZ

@andreas-schwab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

@esmil
Copy link
Collaborator Author

@esmil esmil commented on dd9648b Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreas-schwab Are you saying my patch above fixes the problem? It's just that I spoke to Linus Walleij, and he didn't think the .dt_node_to_map callback was called in parallel. But if the patch above makes a difference then he must be mistaken, so I just want to confirm that it's the patch above that fixes it.

@andreas-schwab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it looks like -rc5 already works without that patch.

@andreas-schwab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it just means the crash is not easy to reproduce.

@esmil
Copy link
Collaborator Author

@esmil esmil commented on dd9648b Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but that would suggest a race, and the .dt_node_to_map callback being run in parallel is the most likely race i can think of right now.

@andreas-schwab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like commit 2acaad3 in opensbi is the key, whatever it does.

@esmil
Copy link
Collaborator Author

@esmil esmil commented on dd9648b Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I'm using plain upstream opensbi, but I recently updated to the latest starfive-tech/beagle_secondBoot, which has the similar commit f5b3e894a2a9a4c13d087d3210a694285a803eb0 and that works great for me.

@esmil
Copy link
Collaborator Author

@esmil esmil commented on dd9648b Aug 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact updating to upstream opensbi fixed a lot of weird crashes for me: starfive-tech/u-boot#17

Please sign in to comment.