| Message ID | cc5c08bd233e7cb7f574dca801fc6a1fc31c4ee1.1657998467.git.sander@svanheule.net |
|---|---|
| State | Superseded |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: MFD for switch core | expand |
On 16-07-2022 21:09, Sander Vanheule wrote: > RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the > switch's ethernet port. This driver allows to address these LEDs and > provides direct control, blink offloading, and switch port status > offloading. > > Since offloading of the generic netdev trigger does not exist, this > driver provides a private trigger which achieves the same and is named > "realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in > /sys/class/leds/$LED, where the requested trigger mode can be written > to. If an unsupported mode is requested, this will be reported to the > user. When the trigger is then activated, the LED will be added to a > group of LEDs with the same trigger conditions. If it is not possible to > add the LED to a group, the user must use an already existing trigger > condition, or use direct LED control with a software trigger. > > Some common modes (i.e. values for "rtl_hw_trigger") are: > - Link present, blink on activity: 1f > - 100M/10M link, blink on activity: f > - 1G link present: 10 > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > --- > .../drivers/pinctrl/pinctrl-rtl-switchcore.c | 370 ++++++++++++++++++ > ...s-add-RTL8380-switch-port-LED-driver.patch | 64 +++ > target/linux/realtek/rtl838x/config-5.10 | 1 + > 3 files changed, 435 insertions(+) > create mode 100644 target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > create mode 100644 target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch > > diff --git a/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > new file mode 100644 > index 000000000000..0caf925989b6 > --- /dev/null > +++ b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > @@ -0,0 +1,370 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +#include <linux/mfd/syscon.h> for ARRAY_SIZE() +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_platform.h> > +#include <linux/pinctrl/pinconf-generic.h> > +#include <linux/pinctrl/pinctrl.h> > +#include <linux/pinctrl/pinmux.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include "core.h" > +#include "pinmux.h" > + > +/** > + * struct rtl_swcore_mux_desc - switchcore pin group information > + * > + * Pins are frequently muxed between alternative functions, but the control > + * bits for the muxes are scattered throught the switchcore's register space. > + * Provide a regmap-based interface to flexibly manage these mux fields, which > + * may vary in size and do not always provide a GPIO function. > + * > + * @name: name to identify the pin group > + * @field_desc: description of the register field with mux control bits > + * @functions: NULL terminated array of function names > + * @pins: array of numbers of the pins in this group > + * @npins: number of pins in this group > + */ > +struct rtl_swcore_mux_desc { > + const char *name; > + struct reg_field field; > + const unsigned int *pins; > + unsigned int npins; > +}; > + > +#define SWITCHCORE_MUX(_name, _field, _pins) { \ > + .name = (_name), \ > + .field = _field, \ > + .pins = (_pins), \ > + .npins = ARRAY_SIZE(_pins), \ > + } > + > +/** > + * struct rtl_swcore_mux_setting - stored mux setting > + * > + * @mux: pointer to the mux descriptor > + * @value: value to write in the mux's register field to apply this setting > + */ > +struct rtl_swcore_mux_setting { > + const struct rtl_swcore_mux_desc *mux; > + unsigned int value; mux_setting? value is so ... undefined ;) > +}; > + > +/** > + * struct rtl_swcore_function_desc - switchcore function information > + * > + * @name: name of this function > + * @settings: list of mux settings that enable this function on said mux > + * @nsettings: length of the @settings list > + */ > +struct rtl_swcore_function_desc { > + const char *name; > + const struct rtl_swcore_mux_setting *settings; > + unsigned int nsettings; > +}; > + > +#define SWITCHCORE_FUNCTION(_name, _settings) { \ > + .name = (_name), \ > + .settings = (_settings), \ > + .nsettings = ARRAY_SIZE(_settings), \ > + } > + > +struct rtl_swcore_config { > + const struct pinctrl_pin_desc *pins; > + unsigned int npins; > + const struct rtl_swcore_function_desc *functions; > + unsigned int nfunctions; > + const struct rtl_swcore_mux_desc **groups; > + unsigned int ngroups; > +}; > + > +struct rtl_swcore_pinctrl { > + struct pinctrl_desc pdesc; > + struct device *dev; > + const struct rtl_swcore_config *config; > + struct regmap_field **mux_fields; > +}; > + > +/* > + * RTL838x chips come in LQFP packages with 216 pins. Pins are indexed > + * counter-clockwise, starting with pin 1 at the bottom left. > + * Map package pin {1..216} to pinctrl pin number {0..215}. > + */ > +static const struct pinctrl_pin_desc rtl8380_swcore_pins[] = { > + /* JTAG pins */ > + PINCTRL_PIN(27, "tck"), > + PINCTRL_PIN(28, "tms"), > + PINCTRL_PIN(29, "tdo"), > + PINCTRL_PIN(30, "tdi"), > + PINCTRL_PIN(31, "ntrst"), maybe a new line to make grouping more clear > + /* aux MDIO bus pins */ Do we have anything other then aux? I mean, these are (external) pins so by definition auxiliary. So it's "just" our MDIO bus isn't it > + PINCTRL_PIN(109, "aux-mdio"), > + PINCTRL_PIN(110, "aux-mdc"), > + /* system LED pin */ > + PINCTRL_PIN(112, "sys-led"), > + /* UART1/SPI slave pins */ > + PINCTRL_PIN(115, "uart1-rx"), > + PINCTRL_PIN(116, "uart1-tx"), you currently name the pads/pins uart1-tx, since we don't have a datasheet, is it worth to use something slightly more descriptive or generic? I know different soc vendors do different things, some use the pin name ("AB1"), some just use the gpioN name, some slash seperated strings ("GPIO112/RG2TXD2/DDRV2") or also ("U8 GMAC0 TXC") some exactly what you do (both underscore and hyphenized, though i'd use the hyphen to connect words, sys-led; underscore to define function uart1_tx (or uart1_tx-sp1_slave)? Anyway, there's a LOT of different flavors, and _personally_ (but hard for us unless we desolder a SoC to do an analsys) I personally a REALLY like the most decriptives ones ("pad name, func1 ... funcn") as it makes it super clear what it is what uses it has, and avoids easy confusion (why ist he spi pin named uart) > +}; > + > +static const unsigned int rtl8380_jtag_pins[] = {27, 28, 29, 30, 31}; > +static const unsigned int rtl8380_aux_mdio_pins[] = {109, 110}; > +static const unsigned int rtl8380_sys_led_pins[] = {112}; > +static const unsigned int rtl8380_uart1_pins[] = {115, 116}; > + > +static const struct rtl_swcore_mux_desc rtl8380_mux_jtag = > + SWITCHCORE_MUX("jtag", REG_FIELD(0x1000, 2, 3), rtl8380_jtag_pins); > + > +static const struct rtl_swcore_mux_desc rtl8380_mux_aux_mdio = > + SWITCHCORE_MUX("aux-mdio", REG_FIELD(0xa0e0, 0, 0), rtl8380_aux_mdio_pins); > + > +static const struct rtl_swcore_mux_desc rtl8380_mux_sys_led = > + SWITCHCORE_MUX("sys-led", REG_FIELD(0xa000, 15, 15), rtl8380_sys_led_pins); > + > +static const struct rtl_swcore_mux_desc rtl8380_mux_uart1 = > + SWITCHCORE_MUX("uart1", REG_FIELD(0x1000, 4, 4), rtl8380_uart1_pins); > + > +static const struct rtl_swcore_mux_desc *rtl8380_groups[] = { > + &rtl8380_mux_jtag, > + &rtl8380_mux_aux_mdio, > + &rtl8380_mux_sys_led, > + &rtl8380_mux_uart1, > +}; > + > +static const struct rtl_swcore_mux_setting rtl8380_gpio_settings[] = { > + {&rtl8380_mux_jtag, 2}, > + {&rtl8380_mux_aux_mdio, 0}, > + {&rtl8380_mux_sys_led, 0}, > +}; > +static const struct rtl_swcore_mux_setting rtl8380_aux_mdio_settings[] = { > + {&rtl8380_mux_aux_mdio, 1}, > +}; > +static const struct rtl_swcore_mux_setting rtl8380_sys_led_settings[] = { > + {&rtl8380_mux_sys_led, 1}, > +}; > +static const struct rtl_swcore_mux_setting rtl8380_uart1_settings[] = { > + {&rtl8380_mux_uart1, 1}, > +}; > +static const struct rtl_swcore_mux_setting rtl8380_spi_slave_settings[] = { > + {&rtl8380_mux_uart1, 0}, > +}; > + > +static const struct rtl_swcore_function_desc rtl8380_functions[] = { > + SWITCHCORE_FUNCTION("gpio", rtl8380_gpio_settings), > + SWITCHCORE_FUNCTION("aux-mdio", rtl8380_aux_mdio_settings), > + SWITCHCORE_FUNCTION("sys-led", rtl8380_sys_led_settings), > + SWITCHCORE_FUNCTION("uart1", rtl8380_uart1_settings), > + SWITCHCORE_FUNCTION("spi-slave", rtl8380_spi_slave_settings), > +}; > + > +static const struct rtl_swcore_config rtl8380_config = { > + .pins = rtl8380_swcore_pins, > + .npins = ARRAY_SIZE(rtl8380_swcore_pins), > + .functions = rtl8380_functions, > + .nfunctions = ARRAY_SIZE(rtl8380_functions), > + .groups = rtl8380_groups, > + .ngroups = ARRAY_SIZE(rtl8380_groups), > +}; > + > +/* > + * RTL839x chips are in BGA packages with 26×26 positions. Board designs number > + * these as 1..26 for the rows, and A..AF for the columns, with position A1 in > + * the bottom left corner. Letters I, O, Q, S, X, and Z are skipped; presumably > + * to avoid ambiguities. > + * This gives a total of 676 positions. Note that not all positions will > + * actually have a pad, and many pads will be used for power. > + * > + * Index pins using (ROW + 26×COL), where ROW and COL mapped as: > + * - ROW: {1..26} -> {0..25} > + * - COL: {A..AF} -> {0..25} > + * > + * Since there are no datasheets available, use a virtual pin range starting at > + * 676 for pins with unknowns positions. When actual pin positions are found > + * (if ever), these can the be mapped to their real values. > + */ > +#define RTL8390_VPIN(num, name) PINCTRL_PIN(26 * 26 + (num), (name)) this is unused code though, right? (i get the RFC nature here) > + > +/* TODO RTL8390 */ > + > +/* TODO RTL9300 */ > + > +/* TODO RTL9310 */ > + > +static int rtl_swcore_group_count(struct pinctrl_dev *pctldev) > +{ > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > + > + return priv->config->ngroups; > +} > + > +static const char * rtl_swcore_group_name(struct pinctrl_dev *pctldev, > + unsigned int selector) > +{ > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > + > + return priv->config->groups[selector]->name; > +}; > + > +static int rtl_swcore_group_pins(struct pinctrl_dev *pctldev, > + unsigned int selector, const unsigned int **pins, > + unsigned int *num_pins) > +{ > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > + > + *pins = priv->config->groups[selector]->pins; > + *num_pins = priv->config->groups[selector]->npins; > + > + return 0; > +} > + > +static int rtl_swcore_set_mux(struct pinctrl_dev *pctldev, > + unsigned int selector, unsigned int group) > +{ > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > + const struct rtl_swcore_function_desc *function; > + const struct rtl_swcore_mux_setting *setting; you sure you do't want to scope anything in the for/if part? > + const struct rtl_swcore_mux_desc *mux; > + unsigned int i; > + > + dev_info(priv->dev, "requesting selector %d, group %d\n", selector, group); > + > + function = &priv->config->functions[selector]; > + mux = priv->config->groups[group]; > + > + for (i = 0; i < function->nsettings; i++) { > + setting = &function->settings[i]; > + if (setting->mux == mux) { > + dev_info(priv->dev, "set mux %s to function %s (%d)\n", > + mux->name, function->name, setting->value); > + return regmap_field_write(priv->mux_fields[group], setting->value); i'm generally not a fan of these 'deeply' hidden return statements; maybe do a goto to jump after the enodev error? > + } > + } > + > + /* Should never hit this, unless something was misconfigured */ dev_err(dev, "something was miss-configured\n"); > + return -ENODEV; > +} > + > +static const struct pinctrl_ops rtl_swcore_pinctrl_ops = { > + .get_groups_count = rtl_swcore_group_count, > + .get_group_name = rtl_swcore_group_name, > + .get_group_pins = rtl_swcore_group_pins, > + .dt_node_to_map = pinconf_generic_dt_node_to_map_all, > + .dt_free_map = pinconf_generic_dt_free_map, > +}; > + > +static const struct pinmux_ops rtl_swcore_pinmux_ops = { > + .get_functions_count = pinmux_generic_get_function_count, > + .get_function_name = pinmux_generic_get_function_name, > + .get_function_groups = pinmux_generic_get_function_groups, > + .set_mux = rtl_swcore_set_mux, > + .strict = true, > +}; > + > +static int rtl_swcore_functions_init(struct pinctrl_dev *pctl, struct rtl_swcore_pinctrl *priv) > +{ > + const struct rtl_swcore_function_desc *function; > + unsigned int ngroups; > + const char **groups; > + unsigned int f_idx; > + unsigned int g_idx; > + > + for (f_idx = 0; f_idx < priv->config->nfunctions; f_idx++) { > + function = &priv->config->functions[f_idx]; > + ngroups = function->nsettings; > + > + dev_info(priv->dev, "found %d groups for function %s\n", ngroups, function->name); shouldn't this be after the add? we THINK all of this will work; but not until after the add we know we actually found them. Can the add function even fail btw? > + > + groups = devm_kcalloc(priv->dev, ngroups, sizeof(*groups), GFP_KERNEL); > + if (!groups) > + return -ENOMEM; > + > + for (g_idx = 0; g_idx < ngroups; g_idx++) > + groups[g_idx] = function->settings[g_idx].mux->name; > + > + pinmux_generic_add_function(pctl, function->name, groups, ngroups, NULL); > + } > + > + return 0; > +} > + > +static const struct of_device_id of_rtl_swcore_pinctrl_match[] = { > + { > + .compatible = "realtek,rtl8380-pinctrl", > + .data = &rtl8380_config, > + }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, of_rtl_swcore_pinctrl_match); > + > +static int rtl_swcore_pinctrl_probe(struct platform_device *pdev) > +{ > + const struct rtl_swcore_config *config; > + struct rtl_swcore_pinctrl *priv; > + struct device *dev = &pdev->dev; > + struct pinctrl_dev *pctldev; > + struct regmap_field *field; > + struct regmap *regmap; > + int mux; > + int err; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + config = of_device_get_match_data(dev); > + if (!config) > + return dev_err_probe(dev, -EINVAL, "no config\n"); + return dev_err_probe(dev, -EINVAL, "unable to match compatible from devicetree\n"); > + > + /* FIXME of_get_parent(dev_of_node(dev)) OR dev_of_node(dev->parent) */ is there even a functional difference between the two? I like what you have now, as that doesn't access struct members > + regmap = device_node_to_regmap(of_get_parent(dev_of_node(dev))); > + if (IS_ERR(regmap)) > + return dev_err_probe(dev, PTR_ERR(regmap), "failed to find parent regmap\n"); > + > + priv->dev = dev; > + priv->config = config; > + priv->pdesc.name = "realtek-switchcore-pinctrl"; > + priv->pdesc.owner = THIS_MODULE; > + priv->pdesc.pctlops = &rtl_swcore_pinctrl_ops; > + priv->pdesc.pmxops = &rtl_swcore_pinmux_ops; > + priv->pdesc.pins = config->pins; > + priv->pdesc.npins = config->npins; > + > + priv->mux_fields = devm_kcalloc(dev, config->ngroups, sizeof(*priv->mux_fields), > + GFP_KERNEL); > + if (!priv->mux_fields) > + return -ENOMEM; > + > + for (mux = 0; mux < config->ngroups; mux++) { > + field = devm_regmap_field_alloc(dev, regmap, config->groups[mux]->field); > + if (IS_ERR(field)) > + return PTR_ERR(field); > + > + priv->mux_fields[mux] = field; > + } > + > + err = devm_pinctrl_register_and_init(dev, &priv->pdesc, priv, &pctldev); > + if (err) > + return dev_err_probe(dev, err, "failed to register\n"); > + > + err = rtl_swcore_functions_init(pctldev, priv); > + if (err) > + return dev_err_probe(dev, err, "failed to generate function list\n"); > + > + err = pinctrl_enable(pctldev); > + if (err) > + return dev_err_probe(dev, err, "failed to enable\n"); > + > + return 0; > +}; > + > +static struct platform_driver rtl_swcore_pinctrl_driver = { > + .driver = { > + .name = "realtek-switchcore-pinctrl", > + .of_match_table = of_rtl_swcore_pinctrl_match > + }, > + .probe = rtl_swcore_pinctrl_probe, > +}; > +module_platform_driver(rtl_swcore_pinctrl_driver); > diff --git a/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch > new file mode 100644 > index 000000000000..5da5a01cdb48 > --- /dev/null > +++ b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch > @@ -0,0 +1,64 @@ > +From d71ec8184236356c50088b00b2417fb142e72bd9 Mon Sep 17 00:00:00 2001 > +From: Sander Vanheule <sander@svanheule.net> > +Date: Sun, 10 Jul 2022 11:31:53 +0200 > +Subject: [PATCH 03/14] leds: add RTL8380 switch port LED driver > +MIME-Version: 1.0 > +Content-Type: text/plain; charset=UTF-8 > +Content-Transfer-Encoding: 8bit > + > +RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the > +switch's ethernet port. This driver allows to address these LEDs and > +provides direct control, blink offloading, and switch port status > +offloading. > + > +Since offloading of the generic netdev trigger does not exist, this > +driver provides a private trigger which achieves the same and is named > +"realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in > +/sys/class/leds/$LED, where the requested trigger mode can be written > +to. If an unsupported mode is requested, this will be reported to the > +user. When the trigger is then activated, the LED will be added to a > +group of LEDs with the same trigger conditions. If it is not possible to > +add the LED to a group, the user must use an already existing trigger > +condition, or use direct LED control with a software trigger. > + > +Some common modes (i.e. values for "rtl_hw_trigger") are: > + - Link present, blink on activity: 1f > + - 100M/10M link, blink on activity: f > + - 1G link present: 10 > + > +Signed-off-by: Sander Vanheule <sander@svanheule.net> > +--- > + drivers/leds/Kconfig | 9 + > + drivers/leds/Makefile | 1 + > + drivers/leds/leds-rtl-switch-port.c | 668 ++++++++++++++++++++++++++++ > + 3 files changed, 678 insertions(+) > + create mode 100644 drivers/leds/leds-rtl-switch-port.c > + > +--- a/drivers/leds/Kconfig > ++++ b/drivers/leds/Kconfig > +@@ -594,6 +594,15 @@ config LEDS_REGULATOR > + help > + This option enables support for regulator driven LEDs. > + > ++config LEDS_RTL_SWITCH_PORT > ++ tristate "Realtek SoC switch port LED support" > ++ depends on LEDS_CLASS > ++ depends on MFD_REALTEK_SWITCHCORE || COMPILE_TEST > ++ select MFD_SYSCON > ++ select REGMAP_MMIO > ++ help > ++ This option enables support for Realtek switch SoC port LEDs. > ++ > + config LEDS_BD2802 > + tristate "LED driver for BD2802 RGB LED" > + depends on LEDS_CLASS > +--- a/drivers/leds/Makefile > ++++ b/drivers/leds/Makefile > +@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm805 > + obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o > + obj-$(CONFIG_LEDS_PWM) += leds-pwm.o > + obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o > ++obj-$(CONFIG_LEDS_RTL_SWITCH_PORT) += leds-rtl-switch-port.o > + obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o > + obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o > + obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o > diff --git a/target/linux/realtek/rtl838x/config-5.10 b/target/linux/realtek/rtl838x/config-5.10 > index 5c69832be41d..a28366a7fd03 100644 > --- a/target/linux/realtek/rtl838x/config-5.10 > +++ b/target/linux/realtek/rtl838x/config-5.10 > @@ -164,6 +164,7 @@ CONFIG_PGTABLE_LEVELS=2 > CONFIG_PHYLIB=y > CONFIG_PHYLINK=y > CONFIG_PINCTRL=y > +CONFIG_PINCTRL_RTL_SWITCHCORE=y > CONFIG_POWER_RESET=y > CONFIG_POWER_RESET_GPIO_RESTART=y > CONFIG_POWER_RESET_SYSCON=y >
On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote: > On 16-07-2022 21:09, Sander Vanheule wrote: > > RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the > > switch's ethernet port. This driver allows to address these LEDs and > > provides direct control, blink offloading, and switch port status > > offloading. > > > > Since offloading of the generic netdev trigger does not exist, this > > driver provides a private trigger which achieves the same and is named > > "realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in > > /sys/class/leds/$LED, where the requested trigger mode can be written > > to. If an unsupported mode is requested, this will be reported to the > > user. When the trigger is then activated, the LED will be added to a > > group of LEDs with the same trigger conditions. If it is not possible to > > add the LED to a group, the user must use an already existing trigger > > condition, or use direct LED control with a software trigger. > > > > Some common modes (i.e. values for "rtl_hw_trigger") are: > > - Link present, blink on activity: 1f > > - 100M/10M link, blink on activity: f > > - 1G link present: 10 > > > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > > --- > > .../drivers/pinctrl/pinctrl-rtl-switchcore.c | 370 ++++++++++++++++++ > > ...s-add-RTL8380-switch-port-LED-driver.patch | 64 +++ > > target/linux/realtek/rtl838x/config-5.10 | 1 + > > 3 files changed, 435 insertions(+) > > create mode 100644 target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl- > > switchcore.c > > create mode 100644 target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch- > > port-LED-driver.patch > > > > diff --git a/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > > b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > > new file mode 100644 > > index 000000000000..0caf925989b6 > > --- /dev/null > > +++ b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c > > @@ -0,0 +1,370 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > + > > +#include <linux/mfd/syscon.h> > for ARRAY_SIZE() > > +#include <linux/kernel.h> > Will add. > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_address.h> > > +#include <linux/of_platform.h> > > +#include <linux/pinctrl/pinconf-generic.h> > > +#include <linux/pinctrl/pinctrl.h> > > +#include <linux/pinctrl/pinmux.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > + > > +#include "core.h" > > +#include "pinmux.h" > > + > > +/** > > + * struct rtl_swcore_mux_desc - switchcore pin group information > > + * > > + * Pins are frequently muxed between alternative functions, but the control > > + * bits for the muxes are scattered throught the switchcore's register space. > > + * Provide a regmap-based interface to flexibly manage these mux fields, which > > + * may vary in size and do not always provide a GPIO function. > > + * > > + * @name: name to identify the pin group > > + * @field_desc: description of the register field with mux control bits > > + * @functions: NULL terminated array of function names > > + * @pins: array of numbers of the pins in this group > > + * @npins: number of pins in this group > > + */ > > +struct rtl_swcore_mux_desc { > > + const char *name; > > + struct reg_field field; > > + const unsigned int *pins; > > + unsigned int npins; > > +}; > > + > > +#define SWITCHCORE_MUX(_name, _field, _pins) { \ > > + .name = (_name), \ > > + .field = _field, \ > > + .pins = (_pins), \ > > + .npins = ARRAY_SIZE(_pins), \ > > + } > > + > > +/** > > + * struct rtl_swcore_mux_setting - stored mux setting > > + * > > + * @mux: pointer to the mux descriptor > > + * @value: value to write in the mux's register field to apply this setting > > + */ > > +struct rtl_swcore_mux_setting { > > + const struct rtl_swcore_mux_desc *mux; > > + unsigned int value; > mux_setting? value is so ... undefined ;) That's because it is very generic, and just contains a register field value The struct is already named mux_setting and mux_setting->mux_setting is too much redundancy IMHO. > > +}; > > + > > +/** > > + * struct rtl_swcore_function_desc - switchcore function information > > + * > > + * @name: name of this function > > + * @settings: list of mux settings that enable this function on said mux > > + * @nsettings: length of the @settings list > > + */ > > +struct rtl_swcore_function_desc { > > + const char *name; > > + const struct rtl_swcore_mux_setting *settings; > > + unsigned int nsettings; > > +}; > > + > > +#define SWITCHCORE_FUNCTION(_name, _settings) { \ > > + .name = (_name), \ > > + .settings = (_settings), \ > > + .nsettings = ARRAY_SIZE(_settings), \ > > + } > > + > > +struct rtl_swcore_config { > > + const struct pinctrl_pin_desc *pins; > > + unsigned int npins; > > + const struct rtl_swcore_function_desc *functions; > > + unsigned int nfunctions; > > + const struct rtl_swcore_mux_desc **groups; > > + unsigned int ngroups; > > +}; > > + > > +struct rtl_swcore_pinctrl { > > + struct pinctrl_desc pdesc; > > + struct device *dev; > > + const struct rtl_swcore_config *config; > > + struct regmap_field **mux_fields; > > +}; > > + > > +/* > > + * RTL838x chips come in LQFP packages with 216 pins. Pins are indexed > > + * counter-clockwise, starting with pin 1 at the bottom left. > > + * Map package pin {1..216} to pinctrl pin number {0..215}. > > + */ > > +static const struct pinctrl_pin_desc rtl8380_swcore_pins[] = { > > + /* JTAG pins */ > > + PINCTRL_PIN(27, "tck"), > > + PINCTRL_PIN(28, "tms"), > > + PINCTRL_PIN(29, "tdo"), > > + PINCTRL_PIN(30, "tdi"), > > + PINCTRL_PIN(31, "ntrst"), > maybe a new line to make grouping more clear > > + /* aux MDIO bus pins */ > Do we have anything other then aux? I mean, these are (external) pins so > by definition auxiliary. So it's "just" our MDIO bus isn't it As explained in the other patch, this isn't for the main MDIO master. I don't even know if the other MDIO master can be muxed as GPIO (or something else). > > + PINCTRL_PIN(109, "aux-mdio"), > > + PINCTRL_PIN(110, "aux-mdc"), > > + /* system LED pin */ > > + PINCTRL_PIN(112, "sys-led"), > > + /* UART1/SPI slave pins */ > > + PINCTRL_PIN(115, "uart1-rx"), > > + PINCTRL_PIN(116, "uart1-tx"), > you currently name the pads/pins uart1-tx, since we don't have a > datasheet, is it worth to use something slightly more descriptive or > generic? I know different soc vendors do different things, some use the > pin name ("AB1"), some just use the gpioN name, some slash seperated > strings ("GPIO112/RG2TXD2/DDRV2") or also ("U8 GMAC0 TXC") some exactly > what you do (both underscore and hyphenized, though i'd use the hyphen > to connect words, sys-led; underscore to define function uart1_tx (or > uart1_tx-sp1_slave)? Anyway, there's a LOT of different flavors, and > _personally_ (but hard for us unless we desolder a SoC to do an analsys) > I personally a REALLY like the most decriptives ones ("pad name, func1 > ... funcn") as it makes it super clear what it is what uses it has, and > avoids easy confusion (why ist he spi pin named uart) But that's some many characters! As long as there's only a few functions (that aren't gpio), we could put them all in the name. Pin index is already provided by the pin number, so we don't have to duplicate that here. > > +}; > > + > > +static const unsigned int rtl8380_jtag_pins[] = {27, 28, 29, 30, 31}; > > +static const unsigned int rtl8380_aux_mdio_pins[] = {109, 110}; > > +static const unsigned int rtl8380_sys_led_pins[] = {112}; > > +static const unsigned int rtl8380_uart1_pins[] = {115, 116}; > > + > > +static const struct rtl_swcore_mux_desc rtl8380_mux_jtag = > > + SWITCHCORE_MUX("jtag", REG_FIELD(0x1000, 2, 3), rtl8380_jtag_pins); > > + > > +static const struct rtl_swcore_mux_desc rtl8380_mux_aux_mdio = > > + SWITCHCORE_MUX("aux-mdio", REG_FIELD(0xa0e0, 0, 0), rtl8380_aux_mdio_pins); > > + > > +static const struct rtl_swcore_mux_desc rtl8380_mux_sys_led = > > + SWITCHCORE_MUX("sys-led", REG_FIELD(0xa000, 15, 15), rtl8380_sys_led_pins); > > + > > +static const struct rtl_swcore_mux_desc rtl8380_mux_uart1 = > > + SWITCHCORE_MUX("uart1", REG_FIELD(0x1000, 4, 4), rtl8380_uart1_pins); > > + > > +static const struct rtl_swcore_mux_desc *rtl8380_groups[] = { > > + &rtl8380_mux_jtag, > > + &rtl8380_mux_aux_mdio, > > + &rtl8380_mux_sys_led, > > + &rtl8380_mux_uart1, > > +}; > > + > > +static const struct rtl_swcore_mux_setting rtl8380_gpio_settings[] = { > > + {&rtl8380_mux_jtag, 2}, > > + {&rtl8380_mux_aux_mdio, 0}, > > + {&rtl8380_mux_sys_led, 0}, > > +}; > > +static const struct rtl_swcore_mux_setting rtl8380_aux_mdio_settings[] = { > > + {&rtl8380_mux_aux_mdio, 1}, > > +}; > > +static const struct rtl_swcore_mux_setting rtl8380_sys_led_settings[] = { > > + {&rtl8380_mux_sys_led, 1}, > > +}; > > +static const struct rtl_swcore_mux_setting rtl8380_uart1_settings[] = { > > + {&rtl8380_mux_uart1, 1}, > > +}; > > +static const struct rtl_swcore_mux_setting rtl8380_spi_slave_settings[] = { > > + {&rtl8380_mux_uart1, 0}, > > +}; > > + > > +static const struct rtl_swcore_function_desc rtl8380_functions[] = { > > + SWITCHCORE_FUNCTION("gpio", rtl8380_gpio_settings), > > + SWITCHCORE_FUNCTION("aux-mdio", rtl8380_aux_mdio_settings), > > + SWITCHCORE_FUNCTION("sys-led", rtl8380_sys_led_settings), > > + SWITCHCORE_FUNCTION("uart1", rtl8380_uart1_settings), > > + SWITCHCORE_FUNCTION("spi-slave", rtl8380_spi_slave_settings), > > +}; > > + > > +static const struct rtl_swcore_config rtl8380_config = { > > + .pins = rtl8380_swcore_pins, > > + .npins = ARRAY_SIZE(rtl8380_swcore_pins), > > + .functions = rtl8380_functions, > > + .nfunctions = ARRAY_SIZE(rtl8380_functions), > > + .groups = rtl8380_groups, > > + .ngroups = ARRAY_SIZE(rtl8380_groups), > > +}; > > + > > +/* > > + * RTL839x chips are in BGA packages with 26×26 positions. Board designs number > > + * these as 1..26 for the rows, and A..AF for the columns, with position A1 in > > + * the bottom left corner. Letters I, O, Q, S, X, and Z are skipped; presumably > > + * to avoid ambiguities. > > + * This gives a total of 676 positions. Note that not all positions will > > + * actually have a pad, and many pads will be used for power. > > + * > > + * Index pins using (ROW + 26×COL), where ROW and COL mapped as: > > + * - ROW: {1..26} -> {0..25} > > + * - COL: {A..AF} -> {0..25} > > + * > > + * Since there are no datasheets available, use a virtual pin range starting at > > + * 676 for pins with unknowns positions. When actual pin positions are found > > + * (if ever), these can the be mapped to their real values. > > + */ > > +#define RTL8390_VPIN(num, name) PINCTRL_PIN(26 * 26 + (num), (name)) > this is unused code though, right? (i get the RFC nature here) It will get used in v2. Didn't feel like stripping it out for the RFC, but sorry for the confusion it caused. > > + > > +/* TODO RTL8390 */ > > + > > +/* TODO RTL9300 */ > > + > > +/* TODO RTL9310 */ > > + > > +static int rtl_swcore_group_count(struct pinctrl_dev *pctldev) > > +{ > > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > > + > > + return priv->config->ngroups; > > +} > > + > > +static const char * rtl_swcore_group_name(struct pinctrl_dev *pctldev, > > + unsigned int selector) > > +{ > > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > > + > > + return priv->config->groups[selector]->name; > > +}; > > + > > +static int rtl_swcore_group_pins(struct pinctrl_dev *pctldev, > > + unsigned int selector, const unsigned int **pins, > > + unsigned int *num_pins) > > +{ > > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > > + > > + *pins = priv->config->groups[selector]->pins; > > + *num_pins = priv->config->groups[selector]->npins; > > + > > + return 0; > > +} > > + > > +static int rtl_swcore_set_mux(struct pinctrl_dev *pctldev, > > + unsigned int selector, unsigned int group) > > +{ > > + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); > > + const struct rtl_swcore_function_desc *function; > > + const struct rtl_swcore_mux_setting *setting; > you sure you do't want to scope anything in the for/if part? Can we do that now? I also prefer keeping the scope of things limited, but the old C standard didn't use to allow for that. > > + const struct rtl_swcore_mux_desc *mux; > > + unsigned int i; > > + > > + dev_info(priv->dev, "requesting selector %d, group %d\n", selector, group); > > + > > + function = &priv->config->functions[selector]; > > + mux = priv->config->groups[group]; > > + > > + for (i = 0; i < function->nsettings; i++) { > > + setting = &function->settings[i]; > > + if (setting->mux == mux) { > > + dev_info(priv->dev, "set mux %s to function %s (%d)\n", > > + mux->name, function->name, setting->value); > > + return regmap_field_write(priv->mux_fields[group], setting- > > >value); > i'm generally not a fan of these 'deeply' hidden return statements; > maybe do a goto to jump after the enodev error? I get that you're not a fan of this, but I don't think a jump would be much better. Maybe this is better (definitely not tested): while (i < function->nsettings && function->settings[i].mux != mux) i++; if (i == function->nsettings) return -ENODEV; return regmap_field_write(priv->mux_fields[group], function->settings[i].value); > > + } > > + } > > + > > + /* Should never hit this, unless something was misconfigured */ > dev_err(dev, "something was miss-configured\n"); > > + return -ENODEV; > > +} > > + > > +static const struct pinctrl_ops rtl_swcore_pinctrl_ops = { > > + .get_groups_count = rtl_swcore_group_count, > > + .get_group_name = rtl_swcore_group_name, > > + .get_group_pins = rtl_swcore_group_pins, > > + .dt_node_to_map = pinconf_generic_dt_node_to_map_all, > > + .dt_free_map = pinconf_generic_dt_free_map, > > +}; > > + > > +static const struct pinmux_ops rtl_swcore_pinmux_ops = { > > + .get_functions_count = pinmux_generic_get_function_count, > > + .get_function_name = pinmux_generic_get_function_name, > > + .get_function_groups = pinmux_generic_get_function_groups, > > + .set_mux = rtl_swcore_set_mux, > > + .strict = true, > > +}; > > + > > +static int rtl_swcore_functions_init(struct pinctrl_dev *pctl, struct > > rtl_swcore_pinctrl *priv) > > +{ > > + const struct rtl_swcore_function_desc *function; > > + unsigned int ngroups; > > + const char **groups; > > + unsigned int f_idx; > > + unsigned int g_idx; > > + > > + for (f_idx = 0; f_idx < priv->config->nfunctions; f_idx++) { > > + function = &priv->config->functions[f_idx]; > > + ngroups = function->nsettings; > > + > > + dev_info(priv->dev, "found %d groups for function %s\n", ngroups, > > function->name); > shouldn't this be after the add? we THINK all of this will work; but not > until after the add we know we actually found them. Can the add function > even fail btw? Development print from times where I wasn't using pinmux_generic_add_function(). Can just be removed I guess. > > + > > + groups = devm_kcalloc(priv->dev, ngroups, sizeof(*groups), > > GFP_KERNEL); > > + if (!groups) > > + return -ENOMEM; > > + > > + for (g_idx = 0; g_idx < ngroups; g_idx++) > > + groups[g_idx] = function->settings[g_idx].mux->name; > > + > > + pinmux_generic_add_function(pctl, function->name, groups, ngroups, > > NULL); > > + } > > + > > + return 0; > > +} > > + > > +static const struct of_device_id of_rtl_swcore_pinctrl_match[] = { > > + { > > + .compatible = "realtek,rtl8380-pinctrl", > > + .data = &rtl8380_config, > > + }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, of_rtl_swcore_pinctrl_match); > > + > > +static int rtl_swcore_pinctrl_probe(struct platform_device *pdev) > > +{ > > + const struct rtl_swcore_config *config; > > + struct rtl_swcore_pinctrl *priv; > > + struct device *dev = &pdev->dev; > > + struct pinctrl_dev *pctldev; > > + struct regmap_field *field; > > + struct regmap *regmap; > > + int mux; > > + int err; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + config = of_device_get_match_data(dev); > > + if (!config) > > + return dev_err_probe(dev, -EINVAL, "no config\n"); > > + return dev_err_probe(dev, -EINVAL, "unable to match compatible from > devicetree\n"); Will improve the log statement > > > + > > + /* FIXME of_get_parent(dev_of_node(dev)) OR dev_of_node(dev->parent) */ > is there even a functional difference between the two? I like what you > have now, as that doesn't access struct members The first takes the parent going through the OF layer, the later going through the device layer. I wouldn't assume these to always be identical. However, since we actually want the DT parent, the current statement should be the right thing. Thanks, Sander > > + regmap = device_node_to_regmap(of_get_parent(dev_of_node(dev))); > > + if (IS_ERR(regmap)) > > + return dev_err_probe(dev, PTR_ERR(regmap), "failed to find parent > > regmap\n"); > > + > > + priv->dev = dev; > > + priv->config = config; > > + priv->pdesc.name = "realtek-switchcore-pinctrl"; > > + priv->pdesc.owner = THIS_MODULE; > > + priv->pdesc.pctlops = &rtl_swcore_pinctrl_ops; > > + priv->pdesc.pmxops = &rtl_swcore_pinmux_ops; > > + priv->pdesc.pins = config->pins; > > + priv->pdesc.npins = config->npins; > > + > > + priv->mux_fields = devm_kcalloc(dev, config->ngroups, sizeof(*priv- > > >mux_fields), > > + GFP_KERNEL); > > + if (!priv->mux_fields) > > + return -ENOMEM; > > + > > + for (mux = 0; mux < config->ngroups; mux++) { > > + field = devm_regmap_field_alloc(dev, regmap, config->groups[mux]- > > >field); > > + if (IS_ERR(field)) > > + return PTR_ERR(field); > > + > > + priv->mux_fields[mux] = field; > > + } > > + > > + err = devm_pinctrl_register_and_init(dev, &priv->pdesc, priv, &pctldev); > > + if (err) > > + return dev_err_probe(dev, err, "failed to register\n"); > > + > > + err = rtl_swcore_functions_init(pctldev, priv); > > + if (err) > > + return dev_err_probe(dev, err, "failed to generate function list\n"); > > + > > + err = pinctrl_enable(pctldev); > > + if (err) > > + return dev_err_probe(dev, err, "failed to enable\n"); > > + > > + return 0; > > +}; > > + > > +static struct platform_driver rtl_swcore_pinctrl_driver = { > > + .driver = { > > + .name = "realtek-switchcore-pinctrl", > > + .of_match_table = of_rtl_swcore_pinctrl_match > > + }, > > + .probe = rtl_swcore_pinctrl_probe, > > +}; > > +module_platform_driver(rtl_swcore_pinctrl_driver); > > diff --git a/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- > > driver.patch b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- > > driver.patch > > new file mode 100644 > > index 000000000000..5da5a01cdb48 > > --- /dev/null > > +++ b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- > > driver.patch > > @@ -0,0 +1,64 @@ > > +From d71ec8184236356c50088b00b2417fb142e72bd9 Mon Sep 17 00:00:00 2001 > > +From: Sander Vanheule <sander@svanheule.net> > > +Date: Sun, 10 Jul 2022 11:31:53 +0200 > > +Subject: [PATCH 03/14] leds: add RTL8380 switch port LED driver > > +MIME-Version: 1.0 > > +Content-Type: text/plain; charset=UTF-8 > > +Content-Transfer-Encoding: 8bit > > + > > +RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the > > +switch's ethernet port. This driver allows to address these LEDs and > > +provides direct control, blink offloading, and switch port status > > +offloading. > > + > > +Since offloading of the generic netdev trigger does not exist, this > > +driver provides a private trigger which achieves the same and is named > > +"realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in > > +/sys/class/leds/$LED, where the requested trigger mode can be written > > +to. If an unsupported mode is requested, this will be reported to the > > +user. When the trigger is then activated, the LED will be added to a > > +group of LEDs with the same trigger conditions. If it is not possible to > > +add the LED to a group, the user must use an already existing trigger > > +condition, or use direct LED control with a software trigger. > > + > > +Some common modes (i.e. values for "rtl_hw_trigger") are: > > + - Link present, blink on activity: 1f > > + - 100M/10M link, blink on activity: f > > + - 1G link present: 10 > > + > > +Signed-off-by: Sander Vanheule <sander@svanheule.net> > > +--- > > + drivers/leds/Kconfig | 9 + > > + drivers/leds/Makefile | 1 + > > + drivers/leds/leds-rtl-switch-port.c | 668 ++++++++++++++++++++++++++++ > > + 3 files changed, 678 insertions(+) > > + create mode 100644 drivers/leds/leds-rtl-switch-port.c > > + > > +--- a/drivers/leds/Kconfig > > ++++ b/drivers/leds/Kconfig > > +@@ -594,6 +594,15 @@ config LEDS_REGULATOR > > + help > > + This option enables support for regulator driven LEDs. > > + > > ++config LEDS_RTL_SWITCH_PORT > > ++ tristate "Realtek SoC switch port LED support" > > ++ depends on LEDS_CLASS > > ++ depends on MFD_REALTEK_SWITCHCORE || COMPILE_TEST > > ++ select MFD_SYSCON > > ++ select REGMAP_MMIO > > ++ help > > ++ This option enables support for Realtek switch SoC port LEDs. > > ++ > > + config LEDS_BD2802 > > + tristate "LED driver for BD2802 RGB LED" > > + depends on LEDS_CLASS > > +--- a/drivers/leds/Makefile > > ++++ b/drivers/leds/Makefile > > +@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm805 > > + obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o > > + obj-$(CONFIG_LEDS_PWM) += leds-pwm.o > > + obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o > > ++obj-$(CONFIG_LEDS_RTL_SWITCH_PORT) += leds-rtl-switch-port.o > > + obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o > > + obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o > > + obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o > > diff --git a/target/linux/realtek/rtl838x/config-5.10 > > b/target/linux/realtek/rtl838x/config-5.10 > > index 5c69832be41d..a28366a7fd03 100644 > > --- a/target/linux/realtek/rtl838x/config-5.10 > > +++ b/target/linux/realtek/rtl838x/config-5.10 > > @@ -164,6 +164,7 @@ CONFIG_PGTABLE_LEVELS=2 > > CONFIG_PHYLIB=y > > CONFIG_PHYLINK=y > > CONFIG_PINCTRL=y > > +CONFIG_PINCTRL_RTL_SWITCHCORE=y > > CONFIG_POWER_RESET=y > > CONFIG_POWER_RESET_GPIO_RESTART=y > > CONFIG_POWER_RESET_SYSCON=y > > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 30-07-2022 00:02, Sander Vanheule wrote: > On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote: >> On 16-07-2022 21:09, Sander Vanheule wrote: >>> RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the >>> switch's ethernet port. This driver allows to address these LEDs and >>> provides direct control, blink offloading, and switch port status >>> offloading. >>> >>> Since offloading of the generic netdev trigger does not exist, this >>> driver provides a private trigger which achieves the same and is named >>> "realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in >>> /sys/class/leds/$LED, where the requested trigger mode can be written >>> to. If an unsupported mode is requested, this will be reported to the >>> user. When the trigger is then activated, the LED will be added to a >>> group of LEDs with the same trigger conditions. If it is not possible to >>> add the LED to a group, the user must use an already existing trigger >>> condition, or use direct LED control with a software trigger. >>> >>> Some common modes (i.e. values for "rtl_hw_trigger") are: >>> - Link present, blink on activity: 1f >>> - 100M/10M link, blink on activity: f >>> - 1G link present: 10 >>> >>> Signed-off-by: Sander Vanheule <sander@svanheule.net> >>> --- >>> .../drivers/pinctrl/pinctrl-rtl-switchcore.c | 370 ++++++++++++++++++ >>> ...s-add-RTL8380-switch-port-LED-driver.patch | 64 +++ >>> target/linux/realtek/rtl838x/config-5.10 | 1 + >>> 3 files changed, 435 insertions(+) >>> create mode 100644 target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl- >>> switchcore.c >>> create mode 100644 target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch- >>> port-LED-driver.patch >>> >>> diff --git a/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c >>> b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c >>> new file mode 100644 >>> index 000000000000..0caf925989b6 >>> --- /dev/null >>> +++ b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c >>> @@ -0,0 +1,370 @@ >>> +// SPDX-License-Identifier: GPL-2.0-or-later >>> + >>> +#include <linux/mfd/syscon.h> >> for ARRAY_SIZE() >> >> +#include <linux/kernel.h> >> > Will add. > >>> +#include <linux/module.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_platform.h> >>> +#include <linux/pinctrl/pinconf-generic.h> >>> +#include <linux/pinctrl/pinctrl.h> >>> +#include <linux/pinctrl/pinmux.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/regmap.h> >>> + >>> +#include "core.h" >>> +#include "pinmux.h" >>> + >>> +/** >>> + * struct rtl_swcore_mux_desc - switchcore pin group information >>> + * >>> + * Pins are frequently muxed between alternative functions, but the control >>> + * bits for the muxes are scattered throught the switchcore's register space. >>> + * Provide a regmap-based interface to flexibly manage these mux fields, which >>> + * may vary in size and do not always provide a GPIO function. >>> + * >>> + * @name: name to identify the pin group >>> + * @field_desc: description of the register field with mux control bits >>> + * @functions: NULL terminated array of function names >>> + * @pins: array of numbers of the pins in this group >>> + * @npins: number of pins in this group >>> + */ >>> +struct rtl_swcore_mux_desc { >>> + const char *name; >>> + struct reg_field field; >>> + const unsigned int *pins; >>> + unsigned int npins; >>> +}; >>> + >>> +#define SWITCHCORE_MUX(_name, _field, _pins) { \ >>> + .name = (_name), \ >>> + .field = _field, \ >>> + .pins = (_pins), \ >>> + .npins = ARRAY_SIZE(_pins), \ >>> + } >>> + >>> +/** >>> + * struct rtl_swcore_mux_setting - stored mux setting >>> + * >>> + * @mux: pointer to the mux descriptor >>> + * @value: value to write in the mux's register field to apply this setting >>> + */ >>> +struct rtl_swcore_mux_setting { >>> + const struct rtl_swcore_mux_desc *mux; >>> + unsigned int value; >> mux_setting? value is so ... undefined ;) > That's because it is very generic, and just contains a register field value > The struct is already named mux_setting and mux_setting->mux_setting is too much > redundancy IMHO. yeah true, but `value` is still a weird name, it must represent 'something' right? In any case remember that naming things is hard ;) but a good name can make quite the difference. I fully agree having mux_setting->setting is dumb, what about rtl_swcore_mux_ctrl->mux_setting? idk; just thinking out-loud here. > >>> +}; >>> + >>> +/** >>> + * struct rtl_swcore_function_desc - switchcore function information >>> + * >>> + * @name: name of this function >>> + * @settings: list of mux settings that enable this function on said mux >>> + * @nsettings: length of the @settings list >>> + */ >>> +struct rtl_swcore_function_desc { >>> + const char *name; >>> + const struct rtl_swcore_mux_setting *settings; >>> + unsigned int nsettings; >>> +}; >>> + >>> +#define SWITCHCORE_FUNCTION(_name, _settings) { \ >>> + .name = (_name), \ >>> + .settings = (_settings), \ >>> + .nsettings = ARRAY_SIZE(_settings), \ >>> + } >>> + >>> +struct rtl_swcore_config { >>> + const struct pinctrl_pin_desc *pins; >>> + unsigned int npins; >>> + const struct rtl_swcore_function_desc *functions; >>> + unsigned int nfunctions; >>> + const struct rtl_swcore_mux_desc **groups; >>> + unsigned int ngroups; >>> +}; >>> + >>> +struct rtl_swcore_pinctrl { >>> + struct pinctrl_desc pdesc; >>> + struct device *dev; >>> + const struct rtl_swcore_config *config; >>> + struct regmap_field **mux_fields; >>> +}; >>> + >>> +/* >>> + * RTL838x chips come in LQFP packages with 216 pins. Pins are indexed >>> + * counter-clockwise, starting with pin 1 at the bottom left. >>> + * Map package pin {1..216} to pinctrl pin number {0..215}. >>> + */ >>> +static const struct pinctrl_pin_desc rtl8380_swcore_pins[] = { >>> + /* JTAG pins */ >>> + PINCTRL_PIN(27, "tck"), >>> + PINCTRL_PIN(28, "tms"), >>> + PINCTRL_PIN(29, "tdo"), >>> + PINCTRL_PIN(30, "tdi"), >>> + PINCTRL_PIN(31, "ntrst"), >> maybe a new line to make grouping more clear >>> + /* aux MDIO bus pins */ >> Do we have anything other then aux? I mean, these are (external) pins so >> by definition auxiliary. So it's "just" our MDIO bus isn't it > As explained in the other patch, this isn't for the main MDIO master. I don't even know if > the other MDIO master can be muxed as GPIO (or something else). Oh for sure true, but in the datasheets, its also never called 'aux-mdio' is it? > >>> + PINCTRL_PIN(109, "aux-mdio"), >>> + PINCTRL_PIN(110, "aux-mdc"), >>> + /* system LED pin */ >>> + PINCTRL_PIN(112, "sys-led"), >>> + /* UART1/SPI slave pins */ >>> + PINCTRL_PIN(115, "uart1-rx"), >>> + PINCTRL_PIN(116, "uart1-tx"), >> you currently name the pads/pins uart1-tx, since we don't have a >> datasheet, is it worth to use something slightly more descriptive or >> generic? I know different soc vendors do different things, some use the >> pin name ("AB1"), some just use the gpioN name, some slash seperated >> strings ("GPIO112/RG2TXD2/DDRV2") or also ("U8 GMAC0 TXC") some exactly >> what you do (both underscore and hyphenized, though i'd use the hyphen >> to connect words, sys-led; underscore to define function uart1_tx (or >> uart1_tx-sp1_slave)? Anyway, there's a LOT of different flavors, and >> _personally_ (but hard for us unless we desolder a SoC to do an analsys) >> I personally a REALLY like the most decriptives ones ("pad name, func1 >> ... funcn") as it makes it super clear what it is what uses it has, and >> avoids easy confusion (why ist he spi pin named uart) > But that's some many characters! As long as there's only a few functions (that aren't > gpio), we could put them all in the name. Pin index is already provided by the pin number, > so we don't have to duplicate that here. Was just listing some examples from other SoC's :) Don't look to hard at the pin number, it's a sequential number only on the rtl838x isn't it, on BGA's the index becomes a matrix, so then you'd have `PINCTRL_PIN(116, "AB1/UART1-TX/I2C-CLK)" or something? > >>> +}; >>> + >>> +static const unsigned int rtl8380_jtag_pins[] = {27, 28, 29, 30, 31}; >>> +static const unsigned int rtl8380_aux_mdio_pins[] = {109, 110}; >>> +static const unsigned int rtl8380_sys_led_pins[] = {112}; >>> +static const unsigned int rtl8380_uart1_pins[] = {115, 116}; >>> + >>> +static const struct rtl_swcore_mux_desc rtl8380_mux_jtag = >>> + SWITCHCORE_MUX("jtag", REG_FIELD(0x1000, 2, 3), rtl8380_jtag_pins); >>> + >>> +static const struct rtl_swcore_mux_desc rtl8380_mux_aux_mdio = >>> + SWITCHCORE_MUX("aux-mdio", REG_FIELD(0xa0e0, 0, 0), rtl8380_aux_mdio_pins); >>> + >>> +static const struct rtl_swcore_mux_desc rtl8380_mux_sys_led = >>> + SWITCHCORE_MUX("sys-led", REG_FIELD(0xa000, 15, 15), rtl8380_sys_led_pins); >>> + >>> +static const struct rtl_swcore_mux_desc rtl8380_mux_uart1 = >>> + SWITCHCORE_MUX("uart1", REG_FIELD(0x1000, 4, 4), rtl8380_uart1_pins); >>> + >>> +static const struct rtl_swcore_mux_desc *rtl8380_groups[] = { >>> + &rtl8380_mux_jtag, >>> + &rtl8380_mux_aux_mdio, >>> + &rtl8380_mux_sys_led, >>> + &rtl8380_mux_uart1, >>> +}; >>> + >>> +static const struct rtl_swcore_mux_setting rtl8380_gpio_settings[] = { >>> + {&rtl8380_mux_jtag, 2}, >>> + {&rtl8380_mux_aux_mdio, 0}, >>> + {&rtl8380_mux_sys_led, 0}, >>> +}; >>> +static const struct rtl_swcore_mux_setting rtl8380_aux_mdio_settings[] = { >>> + {&rtl8380_mux_aux_mdio, 1}, >>> +}; >>> +static const struct rtl_swcore_mux_setting rtl8380_sys_led_settings[] = { >>> + {&rtl8380_mux_sys_led, 1}, >>> +}; >>> +static const struct rtl_swcore_mux_setting rtl8380_uart1_settings[] = { >>> + {&rtl8380_mux_uart1, 1}, >>> +}; >>> +static const struct rtl_swcore_mux_setting rtl8380_spi_slave_settings[] = { >>> + {&rtl8380_mux_uart1, 0}, >>> +}; >>> + >>> +static const struct rtl_swcore_function_desc rtl8380_functions[] = { >>> + SWITCHCORE_FUNCTION("gpio", rtl8380_gpio_settings), >>> + SWITCHCORE_FUNCTION("aux-mdio", rtl8380_aux_mdio_settings), >>> + SWITCHCORE_FUNCTION("sys-led", rtl8380_sys_led_settings), >>> + SWITCHCORE_FUNCTION("uart1", rtl8380_uart1_settings), >>> + SWITCHCORE_FUNCTION("spi-slave", rtl8380_spi_slave_settings), >>> +}; >>> + >>> +static const struct rtl_swcore_config rtl8380_config = { >>> + .pins = rtl8380_swcore_pins, >>> + .npins = ARRAY_SIZE(rtl8380_swcore_pins), >>> + .functions = rtl8380_functions, >>> + .nfunctions = ARRAY_SIZE(rtl8380_functions), >>> + .groups = rtl8380_groups, >>> + .ngroups = ARRAY_SIZE(rtl8380_groups), >>> +}; >>> + >>> +/* >>> + * RTL839x chips are in BGA packages with 26×26 positions. Board designs number >>> + * these as 1..26 for the rows, and A..AF for the columns, with position A1 in >>> + * the bottom left corner. Letters I, O, Q, S, X, and Z are skipped; presumably >>> + * to avoid ambiguities. >>> + * This gives a total of 676 positions. Note that not all positions will >>> + * actually have a pad, and many pads will be used for power. >>> + * >>> + * Index pins using (ROW + 26×COL), where ROW and COL mapped as: >>> + * - ROW: {1..26} -> {0..25} >>> + * - COL: {A..AF} -> {0..25} >>> + * >>> + * Since there are no datasheets available, use a virtual pin range starting at >>> + * 676 for pins with unknowns positions. When actual pin positions are found >>> + * (if ever), these can the be mapped to their real values. >>> + */ >>> +#define RTL8390_VPIN(num, name) PINCTRL_PIN(26 * 26 + (num), (name)) >> this is unused code though, right? (i get the RFC nature here) > It will get used in v2. Didn't feel like stripping it out for the RFC, but sorry for the > confusion it caused. right, no problem; As I only have the 93xx, I actually intend to put some focus on that one, but as discussed before, clk first ;) > >>> + >>> +/* TODO RTL8390 */ >>> + >>> +/* TODO RTL9300 */ >>> + >>> +/* TODO RTL9310 */ >>> + >>> +static int rtl_swcore_group_count(struct pinctrl_dev *pctldev) >>> +{ >>> + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); >>> + >>> + return priv->config->ngroups; >>> +} >>> + >>> +static const char * rtl_swcore_group_name(struct pinctrl_dev *pctldev, >>> + unsigned int selector) >>> +{ >>> + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); >>> + >>> + return priv->config->groups[selector]->name; >>> +}; >>> + >>> +static int rtl_swcore_group_pins(struct pinctrl_dev *pctldev, >>> + unsigned int selector, const unsigned int **pins, >>> + unsigned int *num_pins) >>> +{ >>> + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); >>> + >>> + *pins = priv->config->groups[selector]->pins; >>> + *num_pins = priv->config->groups[selector]->npins; >>> + >>> + return 0; >>> +} >>> + >>> +static int rtl_swcore_set_mux(struct pinctrl_dev *pctldev, >>> + unsigned int selector, unsigned int group) >>> +{ >>> + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); >>> + const struct rtl_swcore_function_desc *function; >>> + const struct rtl_swcore_mux_setting *setting; >> you sure you do't want to scope anything in the for/if part? > Can we do that now? I also prefer keeping the scope of things limited, but the old C > standard didn't use to allow for that. I think scoping variables as locally as possible has been possible for decades, and be done for a _very_ long time in the kernel as well. The only thing that I have seen being avoided was `for (int i = 0; ...)` but even that is now allowed (not randomly creating variables everywhere, but specifically in the for loop context) as a) you can argue its a very local scope, and b) it doesn't pollute your namespace. So yes, we can and should do this imo > >>> + const struct rtl_swcore_mux_desc *mux; >>> + unsigned int i; >>> + >>> + dev_info(priv->dev, "requesting selector %d, group %d\n", selector, group); >>> + >>> + function = &priv->config->functions[selector]; >>> + mux = priv->config->groups[group]; >>> + >>> + for (i = 0; i < function->nsettings; i++) { >>> + setting = &function->settings[i]; >>> + if (setting->mux == mux) { >>> + dev_info(priv->dev, "set mux %s to function %s (%d)\n", >>> + mux->name, function->name, setting->value); >>> + return regmap_field_write(priv->mux_fields[group], setting- >>>> value); >> i'm generally not a fan of these 'deeply' hidden return statements; >> maybe do a goto to jump after the enodev error? > I get that you're not a fan of this, but I don't think a jump would be much better. > > Maybe this is better (definitely not tested): > > while (i < function->nsettings && function->settings[i].mux != mux) > i++; > > if (i == function->nsettings) > return -ENODEV; > > return regmap_field_write(priv->mux_fields[group], function->settings[i].value); Yeah this is a lot better! As for the goto vs the return, the goto still helps to have your single exit point, the early returns on early checks is fine btw, but having a return so deeply nested and hidden just irks a bit, whereas a goto not as much. > > > >>> + } >>> + } >>> + >>> + /* Should never hit this, unless something was misconfigured */ >> dev_err(dev, "something was miss-configured\n"); >>> + return -ENODEV; >>> +} >>> + >>> +static const struct pinctrl_ops rtl_swcore_pinctrl_ops = { >>> + .get_groups_count = rtl_swcore_group_count, >>> + .get_group_name = rtl_swcore_group_name, >>> + .get_group_pins = rtl_swcore_group_pins, >>> + .dt_node_to_map = pinconf_generic_dt_node_to_map_all, >>> + .dt_free_map = pinconf_generic_dt_free_map, >>> +}; >>> + >>> +static const struct pinmux_ops rtl_swcore_pinmux_ops = { >>> + .get_functions_count = pinmux_generic_get_function_count, >>> + .get_function_name = pinmux_generic_get_function_name, >>> + .get_function_groups = pinmux_generic_get_function_groups, >>> + .set_mux = rtl_swcore_set_mux, >>> + .strict = true, >>> +}; >>> + >>> +static int rtl_swcore_functions_init(struct pinctrl_dev *pctl, struct >>> rtl_swcore_pinctrl *priv) >>> +{ >>> + const struct rtl_swcore_function_desc *function; >>> + unsigned int ngroups; >>> + const char **groups; >>> + unsigned int f_idx; >>> + unsigned int g_idx; >>> + >>> + for (f_idx = 0; f_idx < priv->config->nfunctions; f_idx++) { >>> + function = &priv->config->functions[f_idx]; >>> + ngroups = function->nsettings; >>> + >>> + dev_info(priv->dev, "found %d groups for function %s\n", ngroups, >>> function->name); >> shouldn't this be after the add? we THINK all of this will work; but not >> until after the add we know we actually found them. Can the add function >> even fail btw? > Development print from times where I wasn't using pinmux_generic_add_function(). Can just > be removed I guess. > >>> + >>> + groups = devm_kcalloc(priv->dev, ngroups, sizeof(*groups), >>> GFP_KERNEL); >>> + if (!groups) >>> + return -ENOMEM; >>> + >>> + for (g_idx = 0; g_idx < ngroups; g_idx++) >>> + groups[g_idx] = function->settings[g_idx].mux->name; >>> + >>> + pinmux_generic_add_function(pctl, function->name, groups, ngroups, >>> NULL); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static const struct of_device_id of_rtl_swcore_pinctrl_match[] = { >>> + { >>> + .compatible = "realtek,rtl8380-pinctrl", >>> + .data = &rtl8380_config, >>> + }, >>> + { /* sentinel */ } >>> +}; >>> +MODULE_DEVICE_TABLE(of, of_rtl_swcore_pinctrl_match); >>> + >>> +static int rtl_swcore_pinctrl_probe(struct platform_device *pdev) >>> +{ >>> + const struct rtl_swcore_config *config; >>> + struct rtl_swcore_pinctrl *priv; >>> + struct device *dev = &pdev->dev; >>> + struct pinctrl_dev *pctldev; >>> + struct regmap_field *field; >>> + struct regmap *regmap; >>> + int mux; >>> + int err; >>> + >>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >>> + if (!priv) >>> + return -ENOMEM; >>> + >>> + config = of_device_get_match_data(dev); >>> + if (!config) >>> + return dev_err_probe(dev, -EINVAL, "no config\n"); >> + return dev_err_probe(dev, -EINVAL, "unable to match compatible from >> devicetree\n"); > Will improve the log statement > >>> + >>> + /* FIXME of_get_parent(dev_of_node(dev)) OR dev_of_node(dev->parent) */ >> is there even a functional difference between the two? I like what you >> have now, as that doesn't access struct members > The first takes the parent going through the OF layer, the later going through the device > layer. I wouldn't assume these to always be identical. However, since we actually want the > DT parent, the current statement should be the right thing. > > Thanks, > Sander > >>> + regmap = device_node_to_regmap(of_get_parent(dev_of_node(dev))); >>> + if (IS_ERR(regmap)) >>> + return dev_err_probe(dev, PTR_ERR(regmap), "failed to find parent >>> regmap\n"); >>> + >>> + priv->dev = dev; >>> + priv->config = config; >>> + priv->pdesc.name = "realtek-switchcore-pinctrl"; >>> + priv->pdesc.owner = THIS_MODULE; >>> + priv->pdesc.pctlops = &rtl_swcore_pinctrl_ops; >>> + priv->pdesc.pmxops = &rtl_swcore_pinmux_ops; >>> + priv->pdesc.pins = config->pins; >>> + priv->pdesc.npins = config->npins; >>> + >>> + priv->mux_fields = devm_kcalloc(dev, config->ngroups, sizeof(*priv- >>>> mux_fields), >>> + GFP_KERNEL); >>> + if (!priv->mux_fields) >>> + return -ENOMEM; >>> + >>> + for (mux = 0; mux < config->ngroups; mux++) { >>> + field = devm_regmap_field_alloc(dev, regmap, config->groups[mux]- >>>> field); >>> + if (IS_ERR(field)) >>> + return PTR_ERR(field); >>> + >>> + priv->mux_fields[mux] = field; >>> + } >>> + >>> + err = devm_pinctrl_register_and_init(dev, &priv->pdesc, priv, &pctldev); >>> + if (err) >>> + return dev_err_probe(dev, err, "failed to register\n"); >>> + >>> + err = rtl_swcore_functions_init(pctldev, priv); >>> + if (err) >>> + return dev_err_probe(dev, err, "failed to generate function list\n"); >>> + >>> + err = pinctrl_enable(pctldev); >>> + if (err) >>> + return dev_err_probe(dev, err, "failed to enable\n"); >>> + >>> + return 0; >>> +}; >>> + >>> +static struct platform_driver rtl_swcore_pinctrl_driver = { >>> + .driver = { >>> + .name = "realtek-switchcore-pinctrl", >>> + .of_match_table = of_rtl_swcore_pinctrl_match >>> + }, >>> + .probe = rtl_swcore_pinctrl_probe, >>> +}; >>> +module_platform_driver(rtl_swcore_pinctrl_driver); >>> diff --git a/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- >>> driver.patch b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- >>> driver.patch >>> new file mode 100644 >>> index 000000000000..5da5a01cdb48 >>> --- /dev/null >>> +++ b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED- >>> driver.patch >>> @@ -0,0 +1,64 @@ >>> +From d71ec8184236356c50088b00b2417fb142e72bd9 Mon Sep 17 00:00:00 2001 >>> +From: Sander Vanheule <sander@svanheule.net> >>> +Date: Sun, 10 Jul 2022 11:31:53 +0200 >>> +Subject: [PATCH 03/14] leds: add RTL8380 switch port LED driver >>> +MIME-Version: 1.0 >>> +Content-Type: text/plain; charset=UTF-8 >>> +Content-Transfer-Encoding: 8bit >>> + >>> +RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the >>> +switch's ethernet port. This driver allows to address these LEDs and >>> +provides direct control, blink offloading, and switch port status >>> +offloading. >>> + >>> +Since offloading of the generic netdev trigger does not exist, this >>> +driver provides a private trigger which achieves the same and is named >>> +"realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in >>> +/sys/class/leds/$LED, where the requested trigger mode can be written >>> +to. If an unsupported mode is requested, this will be reported to the >>> +user. When the trigger is then activated, the LED will be added to a >>> +group of LEDs with the same trigger conditions. If it is not possible to >>> +add the LED to a group, the user must use an already existing trigger >>> +condition, or use direct LED control with a software trigger. >>> + >>> +Some common modes (i.e. values for "rtl_hw_trigger") are: >>> + - Link present, blink on activity: 1f >>> + - 100M/10M link, blink on activity: f >>> + - 1G link present: 10 >>> + >>> +Signed-off-by: Sander Vanheule <sander@svanheule.net> >>> +--- >>> + drivers/leds/Kconfig | 9 + >>> + drivers/leds/Makefile | 1 + >>> + drivers/leds/leds-rtl-switch-port.c | 668 ++++++++++++++++++++++++++++ >>> + 3 files changed, 678 insertions(+) >>> + create mode 100644 drivers/leds/leds-rtl-switch-port.c >>> + >>> +--- a/drivers/leds/Kconfig >>> ++++ b/drivers/leds/Kconfig >>> +@@ -594,6 +594,15 @@ config LEDS_REGULATOR >>> + help >>> + This option enables support for regulator driven LEDs. >>> + >>> ++config LEDS_RTL_SWITCH_PORT >>> ++ tristate "Realtek SoC switch port LED support" >>> ++ depends on LEDS_CLASS >>> ++ depends on MFD_REALTEK_SWITCHCORE || COMPILE_TEST >>> ++ select MFD_SYSCON >>> ++ select REGMAP_MMIO >>> ++ help >>> ++ This option enables support for Realtek switch SoC port LEDs. >>> ++ >>> + config LEDS_BD2802 >>> + tristate "LED driver for BD2802 RGB LED" >>> + depends on LEDS_CLASS >>> +--- a/drivers/leds/Makefile >>> ++++ b/drivers/leds/Makefile >>> +@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm805 >>> + obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o >>> + obj-$(CONFIG_LEDS_PWM) += leds-pwm.o >>> + obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o >>> ++obj-$(CONFIG_LEDS_RTL_SWITCH_PORT) += leds-rtl-switch-port.o >>> + obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o >>> + obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o >>> + obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o >>> diff --git a/target/linux/realtek/rtl838x/config-5.10 >>> b/target/linux/realtek/rtl838x/config-5.10 >>> index 5c69832be41d..a28366a7fd03 100644 >>> --- a/target/linux/realtek/rtl838x/config-5.10 >>> +++ b/target/linux/realtek/rtl838x/config-5.10 >>> @@ -164,6 +164,7 @@ CONFIG_PGTABLE_LEVELS=2 >>> CONFIG_PHYLIB=y >>> CONFIG_PHYLINK=y >>> CONFIG_PINCTRL=y >>> +CONFIG_PINCTRL_RTL_SWITCHCORE=y >>> CONFIG_POWER_RESET=y >>> CONFIG_POWER_RESET_GPIO_RESTART=y >>> CONFIG_POWER_RESET_SYSCON=y >>> >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c new file mode 100644 index 000000000000..0caf925989b6 --- /dev/null +++ b/target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/pinctrl/pinconf-generic.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#include "core.h" +#include "pinmux.h" + +/** + * struct rtl_swcore_mux_desc - switchcore pin group information + * + * Pins are frequently muxed between alternative functions, but the control + * bits for the muxes are scattered throught the switchcore's register space. + * Provide a regmap-based interface to flexibly manage these mux fields, which + * may vary in size and do not always provide a GPIO function. + * + * @name: name to identify the pin group + * @field_desc: description of the register field with mux control bits + * @functions: NULL terminated array of function names + * @pins: array of numbers of the pins in this group + * @npins: number of pins in this group + */ +struct rtl_swcore_mux_desc { + const char *name; + struct reg_field field; + const unsigned int *pins; + unsigned int npins; +}; + +#define SWITCHCORE_MUX(_name, _field, _pins) { \ + .name = (_name), \ + .field = _field, \ + .pins = (_pins), \ + .npins = ARRAY_SIZE(_pins), \ + } + +/** + * struct rtl_swcore_mux_setting - stored mux setting + * + * @mux: pointer to the mux descriptor + * @value: value to write in the mux's register field to apply this setting + */ +struct rtl_swcore_mux_setting { + const struct rtl_swcore_mux_desc *mux; + unsigned int value; +}; + +/** + * struct rtl_swcore_function_desc - switchcore function information + * + * @name: name of this function + * @settings: list of mux settings that enable this function on said mux + * @nsettings: length of the @settings list + */ +struct rtl_swcore_function_desc { + const char *name; + const struct rtl_swcore_mux_setting *settings; + unsigned int nsettings; +}; + +#define SWITCHCORE_FUNCTION(_name, _settings) { \ + .name = (_name), \ + .settings = (_settings), \ + .nsettings = ARRAY_SIZE(_settings), \ + } + +struct rtl_swcore_config { + const struct pinctrl_pin_desc *pins; + unsigned int npins; + const struct rtl_swcore_function_desc *functions; + unsigned int nfunctions; + const struct rtl_swcore_mux_desc **groups; + unsigned int ngroups; +}; + +struct rtl_swcore_pinctrl { + struct pinctrl_desc pdesc; + struct device *dev; + const struct rtl_swcore_config *config; + struct regmap_field **mux_fields; +}; + +/* + * RTL838x chips come in LQFP packages with 216 pins. Pins are indexed + * counter-clockwise, starting with pin 1 at the bottom left. + * Map package pin {1..216} to pinctrl pin number {0..215}. + */ +static const struct pinctrl_pin_desc rtl8380_swcore_pins[] = { + /* JTAG pins */ + PINCTRL_PIN(27, "tck"), + PINCTRL_PIN(28, "tms"), + PINCTRL_PIN(29, "tdo"), + PINCTRL_PIN(30, "tdi"), + PINCTRL_PIN(31, "ntrst"), + /* aux MDIO bus pins */ + PINCTRL_PIN(109, "aux-mdio"), + PINCTRL_PIN(110, "aux-mdc"), + /* system LED pin */ + PINCTRL_PIN(112, "sys-led"), + /* UART1/SPI slave pins */ + PINCTRL_PIN(115, "uart1-rx"), + PINCTRL_PIN(116, "uart1-tx"), +}; + +static const unsigned int rtl8380_jtag_pins[] = {27, 28, 29, 30, 31}; +static const unsigned int rtl8380_aux_mdio_pins[] = {109, 110}; +static const unsigned int rtl8380_sys_led_pins[] = {112}; +static const unsigned int rtl8380_uart1_pins[] = {115, 116}; + +static const struct rtl_swcore_mux_desc rtl8380_mux_jtag = + SWITCHCORE_MUX("jtag", REG_FIELD(0x1000, 2, 3), rtl8380_jtag_pins); + +static const struct rtl_swcore_mux_desc rtl8380_mux_aux_mdio = + SWITCHCORE_MUX("aux-mdio", REG_FIELD(0xa0e0, 0, 0), rtl8380_aux_mdio_pins); + +static const struct rtl_swcore_mux_desc rtl8380_mux_sys_led = + SWITCHCORE_MUX("sys-led", REG_FIELD(0xa000, 15, 15), rtl8380_sys_led_pins); + +static const struct rtl_swcore_mux_desc rtl8380_mux_uart1 = + SWITCHCORE_MUX("uart1", REG_FIELD(0x1000, 4, 4), rtl8380_uart1_pins); + +static const struct rtl_swcore_mux_desc *rtl8380_groups[] = { + &rtl8380_mux_jtag, + &rtl8380_mux_aux_mdio, + &rtl8380_mux_sys_led, + &rtl8380_mux_uart1, +}; + +static const struct rtl_swcore_mux_setting rtl8380_gpio_settings[] = { + {&rtl8380_mux_jtag, 2}, + {&rtl8380_mux_aux_mdio, 0}, + {&rtl8380_mux_sys_led, 0}, +}; +static const struct rtl_swcore_mux_setting rtl8380_aux_mdio_settings[] = { + {&rtl8380_mux_aux_mdio, 1}, +}; +static const struct rtl_swcore_mux_setting rtl8380_sys_led_settings[] = { + {&rtl8380_mux_sys_led, 1}, +}; +static const struct rtl_swcore_mux_setting rtl8380_uart1_settings[] = { + {&rtl8380_mux_uart1, 1}, +}; +static const struct rtl_swcore_mux_setting rtl8380_spi_slave_settings[] = { + {&rtl8380_mux_uart1, 0}, +}; + +static const struct rtl_swcore_function_desc rtl8380_functions[] = { + SWITCHCORE_FUNCTION("gpio", rtl8380_gpio_settings), + SWITCHCORE_FUNCTION("aux-mdio", rtl8380_aux_mdio_settings), + SWITCHCORE_FUNCTION("sys-led", rtl8380_sys_led_settings), + SWITCHCORE_FUNCTION("uart1", rtl8380_uart1_settings), + SWITCHCORE_FUNCTION("spi-slave", rtl8380_spi_slave_settings), +}; + +static const struct rtl_swcore_config rtl8380_config = { + .pins = rtl8380_swcore_pins, + .npins = ARRAY_SIZE(rtl8380_swcore_pins), + .functions = rtl8380_functions, + .nfunctions = ARRAY_SIZE(rtl8380_functions), + .groups = rtl8380_groups, + .ngroups = ARRAY_SIZE(rtl8380_groups), +}; + +/* + * RTL839x chips are in BGA packages with 26×26 positions. Board designs number + * these as 1..26 for the rows, and A..AF for the columns, with position A1 in + * the bottom left corner. Letters I, O, Q, S, X, and Z are skipped; presumably + * to avoid ambiguities. + * This gives a total of 676 positions. Note that not all positions will + * actually have a pad, and many pads will be used for power. + * + * Index pins using (ROW + 26×COL), where ROW and COL mapped as: + * - ROW: {1..26} -> {0..25} + * - COL: {A..AF} -> {0..25} + * + * Since there are no datasheets available, use a virtual pin range starting at + * 676 for pins with unknowns positions. When actual pin positions are found + * (if ever), these can the be mapped to their real values. + */ +#define RTL8390_VPIN(num, name) PINCTRL_PIN(26 * 26 + (num), (name)) + +/* TODO RTL8390 */ + +/* TODO RTL9300 */ + +/* TODO RTL9310 */ + +static int rtl_swcore_group_count(struct pinctrl_dev *pctldev) +{ + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); + + return priv->config->ngroups; +} + +static const char * rtl_swcore_group_name(struct pinctrl_dev *pctldev, + unsigned int selector) +{ + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); + + return priv->config->groups[selector]->name; +}; + +static int rtl_swcore_group_pins(struct pinctrl_dev *pctldev, + unsigned int selector, const unsigned int **pins, + unsigned int *num_pins) +{ + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); + + *pins = priv->config->groups[selector]->pins; + *num_pins = priv->config->groups[selector]->npins; + + return 0; +} + +static int rtl_swcore_set_mux(struct pinctrl_dev *pctldev, + unsigned int selector, unsigned int group) +{ + struct rtl_swcore_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev); + const struct rtl_swcore_function_desc *function; + const struct rtl_swcore_mux_setting *setting; + const struct rtl_swcore_mux_desc *mux; + unsigned int i; + + dev_info(priv->dev, "requesting selector %d, group %d\n", selector, group); + + function = &priv->config->functions[selector]; + mux = priv->config->groups[group]; + + for (i = 0; i < function->nsettings; i++) { + setting = &function->settings[i]; + if (setting->mux == mux) { + dev_info(priv->dev, "set mux %s to function %s (%d)\n", + mux->name, function->name, setting->value); + return regmap_field_write(priv->mux_fields[group], setting->value); + } + } + + /* Should never hit this, unless something was misconfigured */ + return -ENODEV; +} + +static const struct pinctrl_ops rtl_swcore_pinctrl_ops = { + .get_groups_count = rtl_swcore_group_count, + .get_group_name = rtl_swcore_group_name, + .get_group_pins = rtl_swcore_group_pins, + .dt_node_to_map = pinconf_generic_dt_node_to_map_all, + .dt_free_map = pinconf_generic_dt_free_map, +}; + +static const struct pinmux_ops rtl_swcore_pinmux_ops = { + .get_functions_count = pinmux_generic_get_function_count, + .get_function_name = pinmux_generic_get_function_name, + .get_function_groups = pinmux_generic_get_function_groups, + .set_mux = rtl_swcore_set_mux, + .strict = true, +}; + +static int rtl_swcore_functions_init(struct pinctrl_dev *pctl, struct rtl_swcore_pinctrl *priv) +{ + const struct rtl_swcore_function_desc *function; + unsigned int ngroups; + const char **groups; + unsigned int f_idx; + unsigned int g_idx; + + for (f_idx = 0; f_idx < priv->config->nfunctions; f_idx++) { + function = &priv->config->functions[f_idx]; + ngroups = function->nsettings; + + dev_info(priv->dev, "found %d groups for function %s\n", ngroups, function->name); + + groups = devm_kcalloc(priv->dev, ngroups, sizeof(*groups), GFP_KERNEL); + if (!groups) + return -ENOMEM; + + for (g_idx = 0; g_idx < ngroups; g_idx++) + groups[g_idx] = function->settings[g_idx].mux->name; + + pinmux_generic_add_function(pctl, function->name, groups, ngroups, NULL); + } + + return 0; +} + +static const struct of_device_id of_rtl_swcore_pinctrl_match[] = { + { + .compatible = "realtek,rtl8380-pinctrl", + .data = &rtl8380_config, + }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, of_rtl_swcore_pinctrl_match); + +static int rtl_swcore_pinctrl_probe(struct platform_device *pdev) +{ + const struct rtl_swcore_config *config; + struct rtl_swcore_pinctrl *priv; + struct device *dev = &pdev->dev; + struct pinctrl_dev *pctldev; + struct regmap_field *field; + struct regmap *regmap; + int mux; + int err; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + config = of_device_get_match_data(dev); + if (!config) + return dev_err_probe(dev, -EINVAL, "no config\n"); + + /* FIXME of_get_parent(dev_of_node(dev)) OR dev_of_node(dev->parent) */ + regmap = device_node_to_regmap(of_get_parent(dev_of_node(dev))); + if (IS_ERR(regmap)) + return dev_err_probe(dev, PTR_ERR(regmap), "failed to find parent regmap\n"); + + priv->dev = dev; + priv->config = config; + priv->pdesc.name = "realtek-switchcore-pinctrl"; + priv->pdesc.owner = THIS_MODULE; + priv->pdesc.pctlops = &rtl_swcore_pinctrl_ops; + priv->pdesc.pmxops = &rtl_swcore_pinmux_ops; + priv->pdesc.pins = config->pins; + priv->pdesc.npins = config->npins; + + priv->mux_fields = devm_kcalloc(dev, config->ngroups, sizeof(*priv->mux_fields), + GFP_KERNEL); + if (!priv->mux_fields) + return -ENOMEM; + + for (mux = 0; mux < config->ngroups; mux++) { + field = devm_regmap_field_alloc(dev, regmap, config->groups[mux]->field); + if (IS_ERR(field)) + return PTR_ERR(field); + + priv->mux_fields[mux] = field; + } + + err = devm_pinctrl_register_and_init(dev, &priv->pdesc, priv, &pctldev); + if (err) + return dev_err_probe(dev, err, "failed to register\n"); + + err = rtl_swcore_functions_init(pctldev, priv); + if (err) + return dev_err_probe(dev, err, "failed to generate function list\n"); + + err = pinctrl_enable(pctldev); + if (err) + return dev_err_probe(dev, err, "failed to enable\n"); + + return 0; +}; + +static struct platform_driver rtl_swcore_pinctrl_driver = { + .driver = { + .name = "realtek-switchcore-pinctrl", + .of_match_table = of_rtl_swcore_pinctrl_match + }, + .probe = rtl_swcore_pinctrl_probe, +}; +module_platform_driver(rtl_swcore_pinctrl_driver); diff --git a/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch new file mode 100644 index 000000000000..5da5a01cdb48 --- /dev/null +++ b/target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch @@ -0,0 +1,64 @@ +From d71ec8184236356c50088b00b2417fb142e72bd9 Mon Sep 17 00:00:00 2001 +From: Sander Vanheule <sander@svanheule.net> +Date: Sun, 10 Jul 2022 11:31:53 +0200 +Subject: [PATCH 03/14] leds: add RTL8380 switch port LED driver +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the +switch's ethernet port. This driver allows to address these LEDs and +provides direct control, blink offloading, and switch port status +offloading. + +Since offloading of the generic netdev trigger does not exist, this +driver provides a private trigger which achieves the same and is named +"realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in +/sys/class/leds/$LED, where the requested trigger mode can be written +to. If an unsupported mode is requested, this will be reported to the +user. When the trigger is then activated, the LED will be added to a +group of LEDs with the same trigger conditions. If it is not possible to +add the LED to a group, the user must use an already existing trigger +condition, or use direct LED control with a software trigger. + +Some common modes (i.e. values for "rtl_hw_trigger") are: + - Link present, blink on activity: 1f + - 100M/10M link, blink on activity: f + - 1G link present: 10 + +Signed-off-by: Sander Vanheule <sander@svanheule.net> +--- + drivers/leds/Kconfig | 9 + + drivers/leds/Makefile | 1 + + drivers/leds/leds-rtl-switch-port.c | 668 ++++++++++++++++++++++++++++ + 3 files changed, 678 insertions(+) + create mode 100644 drivers/leds/leds-rtl-switch-port.c + +--- a/drivers/leds/Kconfig ++++ b/drivers/leds/Kconfig +@@ -594,6 +594,15 @@ config LEDS_REGULATOR + help + This option enables support for regulator driven LEDs. + ++config LEDS_RTL_SWITCH_PORT ++ tristate "Realtek SoC switch port LED support" ++ depends on LEDS_CLASS ++ depends on MFD_REALTEK_SWITCHCORE || COMPILE_TEST ++ select MFD_SYSCON ++ select REGMAP_MMIO ++ help ++ This option enables support for Realtek switch SoC port LEDs. ++ + config LEDS_BD2802 + tristate "LED driver for BD2802 RGB LED" + depends on LEDS_CLASS +--- a/drivers/leds/Makefile ++++ b/drivers/leds/Makefile +@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm805 + obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o + obj-$(CONFIG_LEDS_PWM) += leds-pwm.o + obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o ++obj-$(CONFIG_LEDS_RTL_SWITCH_PORT) += leds-rtl-switch-port.o + obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o + obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o + obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o diff --git a/target/linux/realtek/rtl838x/config-5.10 b/target/linux/realtek/rtl838x/config-5.10 index 5c69832be41d..a28366a7fd03 100644 --- a/target/linux/realtek/rtl838x/config-5.10 +++ b/target/linux/realtek/rtl838x/config-5.10 @@ -164,6 +164,7 @@ CONFIG_PGTABLE_LEVELS=2 CONFIG_PHYLIB=y CONFIG_PHYLINK=y CONFIG_PINCTRL=y +CONFIG_PINCTRL_RTL_SWITCHCORE=y CONFIG_POWER_RESET=y CONFIG_POWER_RESET_GPIO_RESTART=y CONFIG_POWER_RESET_SYSCON=y
RTL8380 switch SoCs can control up to 3×28 LEDs to display status of the switch's ethernet port. This driver allows to address these LEDs and provides direct control, blink offloading, and switch port status offloading. Since offloading of the generic netdev trigger does not exist, this driver provides a private trigger which achieves the same and is named "realtek-switchport". Each LED will have a file name "rtl_hw_trigger" in /sys/class/leds/$LED, where the requested trigger mode can be written to. If an unsupported mode is requested, this will be reported to the user. When the trigger is then activated, the LED will be added to a group of LEDs with the same trigger conditions. If it is not possible to add the LED to a group, the user must use an already existing trigger condition, or use direct LED control with a software trigger. Some common modes (i.e. values for "rtl_hw_trigger") are: - Link present, blink on activity: 1f - 100M/10M link, blink on activity: f - 1G link present: 10 Signed-off-by: Sander Vanheule <sander@svanheule.net> --- .../drivers/pinctrl/pinctrl-rtl-switchcore.c | 370 ++++++++++++++++++ ...s-add-RTL8380-switch-port-LED-driver.patch | 64 +++ target/linux/realtek/rtl838x/config-5.10 | 1 + 3 files changed, 435 insertions(+) create mode 100644 target/linux/realtek/files-5.10/drivers/pinctrl/pinctrl-rtl-switchcore.c create mode 100644 target/linux/realtek/patches-5.10/202-leds-add-RTL8380-switch-port-LED-driver.patch