| Message ID | 20240503111436.113089-1-yuklin.soo@starfivetech.com |
|---|---|
| Headers | show |
| Series | Add Pinctrl driver for Starfive JH8100 SoC | expand |
On Fri, May 3, 2024 at 1:14 PM Alex Soo <yuklin.soo@starfivetech.com> wrote: > Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east, > sys_west, sys_gmac, and aon. This patch series adds pinctrl > drivers for these 4 pinctrl domains and this patch series is > depending on the JH8100 base patch series in [1] and [2]. > The relevant dt-binding documentation for each pinctrl domain has > been updated accordingly. > > [1] https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfivetech.com/ > [2] https://lore.kernel.org/lkml/20231206115000.295825-1-jeeheng.sia@starfivetech.com/ v3 is starting to look very nice, why is this patch set still in "RFC"? I would like some proper review from the StarFive maintainers at this point so we can get it finished. Yours, Linus Walleij
> -----Original Message----- > From: Linus Walleij <linus.walleij@linaro.org> > Sent: Monday, May 6, 2024 2:35 PM > To: Yuklin Soo <yuklin.soo@starfivetech.com> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>; Hal Feng > <hal.feng@starfivetech.com>; Leyfoon Tan <leyfoon.tan@starfivetech.com>; > Jianlong Huang <jianlong.huang@starfivetech.com>; Emil Renner Berthing > <kernel@esmil.dk>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Drew Fustini <drew@beagleboard.org>; linux-gpio@vger.kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux- > riscv@lists.infradead.org; Paul Walmsley <paul.walmsley@sifive.com>; Palmer > Dabbelt <palmer@dabbelt.com>; Albert Ou <aou@eecs.berkeley.edu> > Subject: Re: [RFC PATCH v3 0/7] Add Pinctrl driver for Starfive JH8100 SoC > > On Fri, May 3, 2024 at 1:14 PM Alex Soo <yuklin.soo@starfivetech.com> > wrote: > > > Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east, > > sys_west, sys_gmac, and aon. This patch series adds pinctrl drivers > > for these 4 pinctrl domains and this patch series is depending on the > > JH8100 base patch series in [1] and [2]. > > The relevant dt-binding documentation for each pinctrl domain has been > > updated accordingly. > > > > [1] > > https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfi > > vetech.com/ [2] > > https://lore.kernel.org/lkml/20231206115000.295825-1- > jeeheng.sia@starf > > ivetech.com/ > > v3 is starting to look very nice, why is this patch set still in "RFC"? > > I would like some proper review from the StarFive maintainers at this point so > we can get it finished. > > Yours, > Linus Walleij Hi Linus Thanks for reviewing the patches. There is a discussion in another thread about the JH8100 SoC being validated on FPGA/Emulation only now. The suggestion is to send the patches as "RFC" before the real silicon availability. https://patchew.org/linux/20231201121410.95298-1-jeeheng.sia@starfivetech.com/20231201121410.95298-3-jeeheng.sia@starfivetech.com/ Regards Ley Foon
On Mon, May 06, 2024 at 07:31:19AM +0000, Leyfoon Tan wrote: > > > > -----Original Message----- > > From: Linus Walleij <linus.walleij@linaro.org> > > Sent: Monday, May 6, 2024 2:35 PM > > To: Yuklin Soo <yuklin.soo@starfivetech.com> > > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>; Hal Feng > > <hal.feng@starfivetech.com>; Leyfoon Tan <leyfoon.tan@starfivetech.com>; > > Jianlong Huang <jianlong.huang@starfivetech.com>; Emil Renner Berthing > > <kernel@esmil.dk>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > > Drew Fustini <drew@beagleboard.org>; linux-gpio@vger.kernel.org; linux- > > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux- > > riscv@lists.infradead.org; Paul Walmsley <paul.walmsley@sifive.com>; Palmer > > Dabbelt <palmer@dabbelt.com>; Albert Ou <aou@eecs.berkeley.edu> > > Subject: Re: [RFC PATCH v3 0/7] Add Pinctrl driver for Starfive JH8100 SoC > > > > On Fri, May 3, 2024 at 1:14 PM Alex Soo <yuklin.soo@starfivetech.com> > > wrote: > > > > > Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east, > > > sys_west, sys_gmac, and aon. This patch series adds pinctrl drivers > > > for these 4 pinctrl domains and this patch series is depending on the > > > JH8100 base patch series in [1] and [2]. > > > The relevant dt-binding documentation for each pinctrl domain has been > > > updated accordingly. > > > > > > [1] > > > https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfi > > > vetech.com/ [2] > > > https://lore.kernel.org/lkml/20231206115000.295825-1- > > jeeheng.sia@starf > > > ivetech.com/ > > > > v3 is starting to look very nice, why is this patch set still in "RFC"? > > > > I would like some proper review from the StarFive maintainers at this point so > > we can get it finished. > > > > Yours, > > Linus Walleij > > Hi Linus > > Thanks for reviewing the patches. > > There is a discussion in another thread about the JH8100 SoC being validated on FPGA/Emulation only now. The suggestion is to send the patches as "RFC" before the real silicon availability. > > https://patchew.org/linux/20231201121410.95298-1-jeeheng.sia@starfivetech.com/20231201121410.95298-3-jeeheng.sia@starfivetech.com/ I know I said "drivers" in that mail you link to, but I was mostly concerned about binding headers etc, of which I think there are actually none here, so see my reply to this thread a few days ago: https://lore.kernel.org/all/20240503-undress-mantra-e5e46b2f6360@spud/
Fri, May 03, 2024 at 07:14:29PM +0800, Alex Soo kirjoitti: > Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east, > sys_west, sys_gmac, and aon. This patch series adds pinctrl > drivers for these 4 pinctrl domains and this patch series is > depending on the JH8100 base patch series in [1] and [2]. > The relevant dt-binding documentation for each pinctrl domain has > been updated accordingly. I commented one of pinctrl: prefixed patch, but it looks like the same comments are applicable to all of them.
Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east, sys_west, sys_gmac, and aon. This patch series adds pinctrl drivers for these 4 pinctrl domains and this patch series is depending on the JH8100 base patch series in [1] and [2]. The relevant dt-binding documentation for each pinctrl domain has been updated accordingly. [1] https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfivetech.com/ [2] https://lore.kernel.org/lkml/20231206115000.295825-1-jeeheng.sia@starfivetech.com/ --- Changes in v3: - Replace "additionalProperties: false" in dt-bindings docs by "unevaluatedProperties: false" to allow reference to properties in generic pinmux and pincfg schema. - Remove redundant keyword "allOf" from the "compatible" property definition. - Drop all PAD_GPIO*_* macros from dt-bindings header "starfive,jh8100-pinctrl.h" and move them to DTS header "jh8100-pinfunc.h". - Drop "GPOUT_LOW", "GPOUT_HIGH", "GPOEN_ENABLE", "GPOEN_DISABLE", and "GPI_NONE" from dt-bindings header. - Add macros "PAD_SLEW_RATE_FAST"and "PAD_SLEW_RATE_SLOW" to dt-bindings header. - Change the commit message of the main driver and sys_east domain subdriver to inform what SoC they run on, and to explain that the main driver provides common APIs to all domain subdrivers to perform their respective tasks, and how the main driver and domain drivers work together. - Add macros JH8100_SYS_E_NGPIO, JH8100_SYS_W_NGPIO, JH8100_SYS_G_NGPIO, JH8100_AON_NGPIO to header pinctrl-starfive-jh8100.h to represent total number of GPIO_PADs in sys-east, sys-west, sys-gmac, and aon (always-on) domains. - In function jh8100_get_padcfg_base() of main driver, macros: PAD_GPIO47_E, PAD_GPIO15_W, PAD_RGPIO15 are replaced by JH8100_SYS_E_NGPIO, JH8100_SYS_W_NGPIO, and JH8100_AON_NGPIO respectively. - Add function jh8100_padcfg_ds_from_uA() to main driver to convert uA (microamperes) from dts property drive-strength-microamp to drive strength value when setting the pin configuration register. - Add function jh8100_padcfg_ds_to_uA() to main driver to convert drive strength value from pin configuration register to uA (microamperes) when reading pin configuration. - Remove "of_gpio.h" and use only the gpiod interfaces in <linux/gpio/consumer.h>. - Remove cross-include "../core.h" from the main driver and move it to header "pinctrl-starfive-jh8100.h". - Remove cross-include "../pinctrl-utils.h" from the main driver and add the function prototype "pinctrl_utils_free_map()" to header "pinctrl-starfive-jh8100.h". - Remove cross-include "../pinmux.h" from the main driver and add the "pinmux_generic_*" function prototypes to header "pinctrl-starfive-jh8100.h". - Remove cross-include “../pinconf.h” from the main driver and add the function prototype "pinconf_generic_parse_dt_config()" to header "pinctrl-starfive-jh8100.h". - Replace "GPOUT_LOW", "GPOUT_HIGH", "GPOEN_ENABLE", "GPOEN_DISABLE", and "GPI_NONE" in the main driver by constant numbers 0, 1, 0, 1, and 255 respectively. - Update function jh8100_get_padcfg_base() to return base address of pad configuration registers for sys-west and aon domains. - Update function jh8100_set_one_pin_mux() to support pad function selection in sys-west domain. - Update function jh8100_gpio_direction_output() to remove JH8100_PADCFG_BIAS_MASK from mask so that when gpioset configure a pin as output, the pin bias setting (pull-up/pull-down) will not be cleared by the driver. - Remove function jh8100_gpio_irq_setup() and the GPIO wakeup irq enablement in probe function from main driver. It is replaced by function gpiochip_wakeup_irq_setup() in gpiolib core "drivers/gpio/gpiolib.c". The function gpiochip_wakeup_irq_setup() is added as the wakeup gpio irq setup function in the gpiolib core. - The interrupt handler jh8100_gpio_wake_irq_handler() in the main driver is removed. It is replaced by gpio_wake_irq_handler() in gpiolib core "drivers/gpio/gpiolib.c". - Remove the inline function pin_to_hwirq() from driver header file "pinctrl-starfive-jh8100.h". - Remove cross-include "../core.h", “../pinmux.h”, and “../pinconf.h” from the sys-east, sys-west, sys-gmac, and aon domain sub-drivers. - In sys_east domain sub-driver, macros PAD_GPIO0_E through PAD_GPIO47_E are replaced by constant numbers 0 through 47. - In sys_west domain sub-driver, macros PAD_GPIO0_W through PAD_GPIO15_W are replaced by constant numbers 0 through 15. - In AON domain sub-driver, macros PAD_RGPIO0 through PAD_RGPIO15 are replaced by constant numbers 0 through 15. Changes in v2: - Add "(always-on)" to document title to clarify acronym AON. - Replace "drive-strength" by "drive-strength-microamp". - Update "slew-rate" property in sys-east, sys-west, and aon document. - remove redundant "bindings" from commit subject and message. - Change regular expression "-[0-9]+$" to "-grp$" to standardize client node names to end with suffix "-grp" instead of "-<numerical _number>". - Use 4 spaces indentation for DTS examples. - Update DTS examples in sys-east, sys-west, and aon document with client driver pinmuxing. - Remove redundant syscon and gmac macros from dt-binding header file. - Remove redundant register macros from dt-binding header file. - Add "wakeup-gpios" and "wakeup-source" to aon document. - Add "gpio-line-names" to sys-east and sys-west document. - Update the description of syscon register usage in each document. - Update sys-gmac and aon document with information of GMAC voltage. reference syscon and GMAC pad syscon. - Fix the pinctrl device nodes compatible string too long issue. - Move all common codes from subdrivers to the main driver. - Change the commit log to "add main and sys_east driver" to indicate the commit of both main and sys-east driver. - Turn pin_to_hwirq macro to a static inline function to hide gpio internal detail, and also, for easier code readability. - Change "JH8100_PADCFG_BIAS" to "JH8100_PADCFG_BIAS_MASK". - Change "#define JH8100_PADCFG_DS_4MA BIT(1)" to #define JH8100_PADCFG_DS_4MA (1U << 1)". - Replace "jh8100_gpio_request" by "pinctrl_gpio_request". - Replace "jh8100_gpio_free" by "pinctrl_gpio_free". - Replace "jh8100_gpio_set_config" by "gpiochip_generic_config". - Use irq_print_chip function to display irqchip name to user space. - Use girq to represent GPIO interrupt controller. - Update code to ensure wakeup-gpios is always an input line. - Remove the jh8100_gpio_add_pin_ranges function and use gpio-ranges in device tree to provide information for GPIO core to add pin range for each pinctrl. - Change "StarFive GPIO chip registered" to "StarFive JH8100 GPIO chip registered". --- Alex Soo (7): dt-bindings: pinctrl: starfive: Add JH8100 pinctrl pinctrl: starfive: jh8100: add main driver and sys_east domain sub-driver pinctrl: starfive: jh8100: add sys_west domain sub-driver pinctrl: starfive: jh8100: add sys_gmac domain sub-driver pinctrl: starfive: jh8100: add AON domain sub-driver gpiolib: enable GPIO interrupt to wake up a system from sleep riscv: dts: starfive: jh8100: add pinctrl device tree nodes .../pinctrl/starfive,jh8100-aon-pinctrl.yaml | 260 ++++ .../starfive,jh8100-sys-east-pinctrl.yaml | 222 ++++ .../starfive,jh8100-sys-gmac-pinctrl.yaml | 162 +++ .../starfive,jh8100-sys-west-pinctrl.yaml | 219 ++++ MAINTAINERS | 7 + arch/riscv/boot/dts/starfive/jh8100-evb.dts | 7 + arch/riscv/boot/dts/starfive/jh8100-pinfunc.h | 504 ++++++++ arch/riscv/boot/dts/starfive/jh8100.dtsi | 46 + drivers/gpio/gpiolib.c | 87 ++ drivers/pinctrl/starfive/Kconfig | 59 + drivers/pinctrl/starfive/Makefile | 6 + .../starfive/pinctrl-starfive-jh8100-aon.c | 150 +++ .../pinctrl-starfive-jh8100-sys-east.c | 220 ++++ .../pinctrl-starfive-jh8100-sys-gmac.c | 89 ++ .../pinctrl-starfive-jh8100-sys-west.c | 164 +++ .../starfive/pinctrl-starfive-jh8100.c | 1103 +++++++++++++++++ .../starfive/pinctrl-starfive-jh8100.h | 125 ++ .../pinctrl/starfive,jh8100-pinctrl.h | 13 + 18 files changed, 3443 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-aon-pinctrl.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-east-pinctrl.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-gmac-pinctrl.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-west-pinctrl.yaml create mode 100644 arch/riscv/boot/dts/starfive/jh8100-pinfunc.h create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-aon.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-east.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-gmac.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-west.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.h create mode 100644 include/dt-bindings/pinctrl/starfive,jh8100-pinctrl.h