Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Christian Marangi]

On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> Preface (new for resend):
> 
> This is a resend of a patch I'd sent a couple years back. At that time,
> I was told to wait for hardware-offloaded LEDS. It looks like that time
> has finally come, so I've changed this from PATCH down to an RFC to make
> sure this is the right approach for the framework.
> 
> Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> hardware-offloaded LEDs based on network activity. This is currenty
> managed by way of pinctrl-microchip-sgpio (and this current patch).
> 
> The purpose of this resend is two-fold. First, to come up with an idea
> of how this pinctrl-microchip-sgpio module can fit in with the new
> hardware-offloaded netdev triggers Christian Marangi recently added. Is
> this something that should be in the pinctrl module itself? Or should
> there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> add?
>

I'm a bit out of the loop on what magic OEM did to make LED work on
ocelot but I feel an ocelot_leds submodule is needed.

To correctly supports the hw many API needs to be defined and for switch
I would stick with how things are done with qca8k, codewise and DT wise
(with how LEDs are defined in DT)

Ideally the feature for MAC will be generilized and added to the DSA ops
struct, so having things in the DSA driver would make the migration
easier.

> The second reason is maybe there's someone out there who might also be
> considering implementing this. This might be a good starting point if
> someone is eager to get coding. On my priority list, this is quite low
> so I'll get to it eventually, but maybe not even in this dev cycle.
> That's why I'm including the original patch.
> 
> 
> Any suggestions on how to approach this problem are welcome.
> 
> 
> 
> 
> (You can probably stop reading here)
> 
> 
> Original Header:
> 
> Expose a debugfs / devicetree interface for Microsemi SGPIO controllers.
> By writing values of 2-5, the SGPIO pins can be configured for either
> automatic blinking or activity.
> 
> The implementation is modeled after the code in
> /drivers/pinctrl/pinctrl-ocelot.c.
> 
> I have only tested this with currently out-of-tree patches for the
> VSC7512 that I hope to get in soon. They are not needed for VSC7513 /
> VSC7514, SPARX5, or LUTON - but I don't have any hardware to test.
> 
> Of note: the 7512 chip has a discrepancy between the datasheet and the
> registers. The datahseet claims 20Hz blink default frequency, the
> registers claim 5 Hz default frequency for BMODE_0. I override the
> OCELOT registers to correct for this. I don't know if that is needed for
> LUTON or SPARX, but having two blink modes at the same frequency isn't
> beneficial. As such, I make the blink modes match the 5Hz / 20Hz for the
> two modes.
> 
> Tested with VSC7512 by way of:
> echo SGPIO_O_p1b0 {blink0,blink1,activity0,activity1} > 
> /sys/kernel/debug/pinctrl/pinctrl-sgpio-pinctrl-sgpio-output/pinmux-select
> 
> LEDs blink!
> 
> 
> Colin Foster (1):
>   pinctrl: microchip-sgpio: add activity and blink functionality
> 
>  drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 5 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Christian Marangi]

On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> Preface (new for resend):
> 
> This is a resend of a patch I'd sent a couple years back. At that time,
> I was told to wait for hardware-offloaded LEDS. It looks like that time
> has finally come, so I've changed this from PATCH down to an RFC to make
> sure this is the right approach for the framework.
> 
> Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> hardware-offloaded LEDs based on network activity. This is currenty
> managed by way of pinctrl-microchip-sgpio (and this current patch).
> 
> The purpose of this resend is two-fold. First, to come up with an idea
> of how this pinctrl-microchip-sgpio module can fit in with the new
> hardware-offloaded netdev triggers Christian Marangi recently added. Is
> this something that should be in the pinctrl module itself? Or should
> there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> add?
>

I'm a bit out of the loop on what magic OEM did to make LED work on
ocelot but I feel an ocelot_leds submodule is needed.

To correctly supports the hw many API needs to be defined and for switch
I would stick with how things are done with qca8k, codewise and DT wise
(with how LEDs are defined in DT)

Ideally the feature for MAC will be generilized and added to the DSA ops
struct, so having things in the DSA driver would make the migration
easier.

> The second reason is maybe there's someone out there who might also be
> considering implementing this. This might be a good starting point if
> someone is eager to get coding. On my priority list, this is quite low
> so I'll get to it eventually, but maybe not even in this dev cycle.
> That's why I'm including the original patch.
> 
> 
> Any suggestions on how to approach this problem are welcome.
> 
> 
> 
> 
> (You can probably stop reading here)
> 
> 
> Original Header:
> 
> Expose a debugfs / devicetree interface for Microsemi SGPIO controllers.
> By writing values of 2-5, the SGPIO pins can be configured for either
> automatic blinking or activity.
> 
> The implementation is modeled after the code in
> /drivers/pinctrl/pinctrl-ocelot.c.
> 
> I have only tested this with currently out-of-tree patches for the
> VSC7512 that I hope to get in soon. They are not needed for VSC7513 /
> VSC7514, SPARX5, or LUTON - but I don't have any hardware to test.
> 
> Of note: the 7512 chip has a discrepancy between the datasheet and the
> registers. The datahseet claims 20Hz blink default frequency, the
> registers claim 5 Hz default frequency for BMODE_0. I override the
> OCELOT registers to correct for this. I don't know if that is needed for
> LUTON or SPARX, but having two blink modes at the same frequency isn't
> beneficial. As such, I make the blink modes match the 5Hz / 20Hz for the
> two modes.
> 
> Tested with VSC7512 by way of:
> echo SGPIO_O_p1b0 {blink0,blink1,activity0,activity1} > 
> /sys/kernel/debug/pinctrl/pinctrl-sgpio-pinctrl-sgpio-output/pinmux-select
> 
> LEDs blink!
> 
> 
> Colin Foster (1):
>   pinctrl: microchip-sgpio: add activity and blink functionality
> 
>  drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 5 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
	Ansuel

[RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Colin Foster]

Preface (new for resend):

This is a resend of a patch I'd sent a couple years back. At that time,
I was told to wait for hardware-offloaded LEDS. It looks like that time
has finally come, so I've changed this from PATCH down to an RFC to make
sure this is the right approach for the framework.

Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
hardware-offloaded LEDs based on network activity. This is currenty
managed by way of pinctrl-microchip-sgpio (and this current patch).

The purpose of this resend is two-fold. First, to come up with an idea
of how this pinctrl-microchip-sgpio module can fit in with the new
hardware-offloaded netdev triggers Christian Marangi recently added. Is
this something that should be in the pinctrl module itself? Or should
there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
add?

The second reason is maybe there's someone out there who might also be
considering implementing this. This might be a good starting point if
someone is eager to get coding. On my priority list, this is quite low
so I'll get to it eventually, but maybe not even in this dev cycle.
That's why I'm including the original patch.


Any suggestions on how to approach this problem are welcome.




(You can probably stop reading here)


Original Header:

Expose a debugfs / devicetree interface for Microsemi SGPIO controllers.
By writing values of 2-5, the SGPIO pins can be configured for either
automatic blinking or activity.

The implementation is modeled after the code in
/drivers/pinctrl/pinctrl-ocelot.c.

I have only tested this with currently out-of-tree patches for the
VSC7512 that I hope to get in soon. They are not needed for VSC7513 /
VSC7514, SPARX5, or LUTON - but I don't have any hardware to test.

Of note: the 7512 chip has a discrepancy between the datasheet and the
registers. The datahseet claims 20Hz blink default frequency, the
registers claim 5 Hz default frequency for BMODE_0. I override the
OCELOT registers to correct for this. I don't know if that is needed for
LUTON or SPARX, but having two blink modes at the same frequency isn't
beneficial. As such, I make the blink modes match the 5Hz / 20Hz for the
two modes.

Tested with VSC7512 by way of:
echo SGPIO_O_p1b0 {blink0,blink1,activity0,activity1} > 
/sys/kernel/debug/pinctrl/pinctrl-sgpio-pinctrl-sgpio-output/pinmux-select

LEDs blink!


Colin Foster (1):
  pinctrl: microchip-sgpio: add activity and blink functionality

 drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
 1 file changed, 130 insertions(+), 5 deletions(-)

-- 
2.25.1

[RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Colin Foster]

Preface (new for resend):

This is a resend of a patch I'd sent a couple years back. At that time,
I was told to wait for hardware-offloaded LEDS. It looks like that time
has finally come, so I've changed this from PATCH down to an RFC to make
sure this is the right approach for the framework.

Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
hardware-offloaded LEDs based on network activity. This is currenty
managed by way of pinctrl-microchip-sgpio (and this current patch).

The purpose of this resend is two-fold. First, to come up with an idea
of how this pinctrl-microchip-sgpio module can fit in with the new
hardware-offloaded netdev triggers Christian Marangi recently added. Is
this something that should be in the pinctrl module itself? Or should
there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
add?

The second reason is maybe there's someone out there who might also be
considering implementing this. This might be a good starting point if
someone is eager to get coding. On my priority list, this is quite low
so I'll get to it eventually, but maybe not even in this dev cycle.
That's why I'm including the original patch.


Any suggestions on how to approach this problem are welcome.




(You can probably stop reading here)


Original Header:

Expose a debugfs / devicetree interface for Microsemi SGPIO controllers.
By writing values of 2-5, the SGPIO pins can be configured for either
automatic blinking or activity.

The implementation is modeled after the code in
/drivers/pinctrl/pinctrl-ocelot.c.

I have only tested this with currently out-of-tree patches for the
VSC7512 that I hope to get in soon. They are not needed for VSC7513 /
VSC7514, SPARX5, or LUTON - but I don't have any hardware to test.

Of note: the 7512 chip has a discrepancy between the datasheet and the
registers. The datahseet claims 20Hz blink default frequency, the
registers claim 5 Hz default frequency for BMODE_0. I override the
OCELOT registers to correct for this. I don't know if that is needed for
LUTON or SPARX, but having two blink modes at the same frequency isn't
beneficial. As such, I make the blink modes match the 5Hz / 20Hz for the
two modes.

Tested with VSC7512 by way of:
echo SGPIO_O_p1b0 {blink0,blink1,activity0,activity1} > 
/sys/kernel/debug/pinctrl/pinctrl-sgpio-pinctrl-sgpio-output/pinmux-select

LEDs blink!


Colin Foster (1):
  pinctrl: microchip-sgpio: add activity and blink functionality

 drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
 1 file changed, 130 insertions(+), 5 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Colin Foster]

Add additional functions - two blink and two activity, for each SGPIO
output.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
 1 file changed, 130 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c
index 8e081c90bdb2..e3230e5dedc0 100644
--- a/drivers/pinctrl/pinctrl-microchip-sgpio.c
+++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c
@@ -51,6 +51,15 @@ enum {
 	SGPIO_FLAGS_HAS_IRQ	= BIT(0),
 };
 
+enum {
+	FUNC_GPIO,
+	FUNC_BLINK0,
+	FUNC_BLINK1,
+	FUNC_ACTIVITY0,
+	FUNC_ACTIVITY1,
+	FUNC_MAX,
+};
+
 struct sgpio_properties {
 	int arch;
 	int flags;
@@ -60,16 +69,22 @@ struct sgpio_properties {
 #define SGPIO_LUTON_AUTO_REPEAT  BIT(5)
 #define SGPIO_LUTON_PORT_WIDTH   GENMASK(3, 2)
 #define SGPIO_LUTON_CLK_FREQ     GENMASK(11, 0)
+#define SGPIO_LUTON_SIO_BMODE_0	 GENMASK(21, 20)
+#define SGPIO_LUTON_SIO_BMODE_1	 GENMASK(19, 18)
 #define SGPIO_LUTON_BIT_SOURCE   GENMASK(11, 0)
 
 #define SGPIO_OCELOT_AUTO_REPEAT BIT(10)
 #define SGPIO_OCELOT_PORT_WIDTH  GENMASK(8, 7)
 #define SGPIO_OCELOT_CLK_FREQ    GENMASK(19, 8)
+#define SGPIO_OCELOT_SIO_BMODE_0 GENMASK(20, 19)
+#define SGPIO_OCELOT_SIO_BMODE_1 GENMASK(22, 21)
 #define SGPIO_OCELOT_BIT_SOURCE  GENMASK(23, 12)
 
 #define SGPIO_SPARX5_AUTO_REPEAT BIT(6)
 #define SGPIO_SPARX5_PORT_WIDTH  GENMASK(4, 3)
 #define SGPIO_SPARX5_CLK_FREQ    GENMASK(19, 8)
+#define SGPIO_SPARX5_SIO_BMODE_0 GENMASK(16, 15)
+#define SGPIO_SPARX5_SIO_BMODE_1 GENMASK(18, 17)
 #define SGPIO_SPARX5_BIT_SOURCE  GENMASK(23, 12)
 
 #define SGPIO_MASTER_INTR_ENA    BIT(0)
@@ -98,22 +113,46 @@ static const struct sgpio_properties properties_sparx5 = {
 	.regoff = { 0x00, 0x06, 0x26, 0x04, 0x05, 0x2a, 0x32, 0x3a, 0x3e, 0x42 },
 };
 
-static const char * const functions[] = { "gpio" };
+static const char * const function_names[] = {
+	[FUNC_GPIO] = "gpio",
+	[FUNC_BLINK0] = "blink0",
+	[FUNC_BLINK1] = "blink1",
+	[FUNC_ACTIVITY0] = "activity0",
+	[FUNC_ACTIVITY1] = "activity1",
+};
+
+static const int function_values[] = {
+	[FUNC_GPIO] = 0,
+	[FUNC_BLINK0] = 2,
+	[FUNC_BLINK1] = 3,
+	[FUNC_ACTIVITY0] = 4,
+	[FUNC_ACTIVITY1] = 5,
+};
+
+struct sgpio_pmx_func {
+	const char **groups;
+	unsigned int ngroups;
+};
 
 struct sgpio_bank {
 	struct sgpio_priv *priv;
 	bool is_input;
 	struct gpio_chip gpio;
 	struct pinctrl_desc pctl_desc;
+	struct sgpio_pmx_func func[FUNC_MAX];
+	struct pinctrl_pin_desc *pins;
 };
 
 struct sgpio_priv {
 	struct device *dev;
 	struct sgpio_bank in;
 	struct sgpio_bank out;
+	int ngpios;
 	u32 bitcount;
 	u32 ports;
 	u32 clock;
+	u32 bmode0;
+	u32 bmode1;
 	struct regmap *regs;
 	const struct sgpio_properties *properties;
 };
@@ -223,6 +262,32 @@ static inline void sgpio_configure_clock(struct sgpio_priv *priv, u32 clkfrq)
 	sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0, clr, set);
 }
 
+static inline void sgpio_configure_blink_modes(struct sgpio_priv *priv)
+{
+	u32 clr, set;
+
+	switch (priv->properties->arch) {
+	case SGPIO_ARCH_LUTON:
+		clr = SGPIO_LUTON_SIO_BMODE_0 | SGPIO_LUTON_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_LUTON_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_LUTON_SIO_BMODE_1, priv->bmode1);
+		break;
+	case SGPIO_ARCH_OCELOT:
+		clr = SGPIO_OCELOT_SIO_BMODE_0 | SGPIO_OCELOT_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_OCELOT_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_OCELOT_SIO_BMODE_1, priv->bmode1);
+		break;
+	case SGPIO_ARCH_SPARX5:
+		clr = SGPIO_SPARX5_SIO_BMODE_0 | SGPIO_SPARX5_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_SPARX5_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_SPARX5_SIO_BMODE_1, priv->bmode1);
+		break;
+	default:
+		return;
+	}
+	sgpio_clrsetbits(priv, REG_SIO_CONFIG, 0, clr, set);
+}
+
 static void sgpio_output_set(struct sgpio_priv *priv,
 			     struct sgpio_port_addr *addr,
 			     int value)
@@ -352,13 +417,18 @@ static const struct pinconf_ops sgpio_confops = {
 
 static int sgpio_get_functions_count(struct pinctrl_dev *pctldev)
 {
-	return 1;
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+
+	if (bank->is_input)
+		return 1;
+	else
+		return ARRAY_SIZE(function_names);
 }
 
 static const char *sgpio_get_function_name(struct pinctrl_dev *pctldev,
 					   unsigned int function)
 {
-	return functions[0];
+	return function_names[function];
 }
 
 static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
@@ -366,8 +436,10 @@ static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
 				     const char *const **groups,
 				     unsigned *const num_groups)
 {
-	*groups  = functions;
-	*num_groups = ARRAY_SIZE(functions);
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups  = bank->func[function].groups;
+	*num_groups = bank->func[function].ngroups;
 
 	return 0;
 }
@@ -375,6 +447,15 @@ static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
 static int sgpio_pinmux_set_mux(struct pinctrl_dev *pctldev,
 				unsigned int selector, unsigned int group)
 {
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+	struct sgpio_priv *priv = bank->priv;
+	struct sgpio_port_addr addr;
+	int f;
+
+	f = function_values[selector];
+	sgpio_pin_to_addr(priv, group, &addr);
+	sgpio_output_set(priv, &addr, f);
+
 	return 0;
 }
 
@@ -693,6 +774,30 @@ static void sgpio_irq_handler(struct irq_desc *desc)
 	}
 }
 
+static int sgpio_create_group_func_map(struct device *dev,
+				       struct sgpio_bank *bank)
+{
+	struct sgpio_priv *priv = bank->priv;
+	int f, i;
+
+	if (bank->is_input)
+		return 0;
+
+	for (f = 0; f < FUNC_MAX; f++) {
+		bank->func[f].ngroups = priv->ngpios;
+		bank->func[f].groups = devm_kcalloc(dev, priv->ngpios,
+						    sizeof(char *), GFP_KERNEL);
+
+		if (!bank->func[f].groups)
+			return -ENOMEM;
+
+		for (i = 0; i < priv->ngpios; i++)
+			bank->func[f].groups[i] = bank->pins[i].name;
+	}
+
+	return 0;
+}
+
 static int microchip_sgpio_register_bank(struct device *dev,
 					 struct sgpio_priv *priv,
 					 struct fwnode_handle *fwnode,
@@ -716,6 +821,7 @@ static int microchip_sgpio_register_bank(struct device *dev,
 		ngpios = 64;
 	}
 
+	priv->ngpios = ngpios;
 	priv->bitcount = ngpios / SGPIO_BITS_PER_WORD;
 	if (priv->bitcount > SGPIO_MAX_BITS) {
 		dev_err(dev, "Bit width exceeds maximum (%d)\n",
@@ -738,6 +844,7 @@ static int microchip_sgpio_register_bank(struct device *dev,
 
 	pctl_desc->npins = ngpios;
 	pctl_desc->pins = pins;
+	bank->pins = pins;
 
 	for (i = 0; i < ngpios; i++) {
 		struct sgpio_port_addr addr;
@@ -753,6 +860,12 @@ static int microchip_sgpio_register_bank(struct device *dev,
 			return -ENOMEM;
 	}
 
+	ret = sgpio_create_group_func_map(dev, bank);
+	if (ret) {
+		dev_err(dev, "Unable to create group func map.\n");
+		return ret;
+	}
+
 	pctldev = devm_pinctrl_register(dev, pctl_desc, bank);
 	if (IS_ERR(pctldev))
 		return dev_err_probe(dev, PTR_ERR(pctldev), "Failed to register pinctrl\n");
@@ -895,6 +1008,18 @@ static int microchip_sgpio_probe(struct platform_device *pdev)
 		sgpio_writel(priv, 0, REG_PORT_CONFIG, port);
 	sgpio_writel(priv, priv->ports, REG_PORT_ENABLE, 0);
 
+	/*
+	 * The datasheet and register definitions contradict themselves, at
+	 * least for the VSC7512. The Datasheet Revision 4.2 describes both
+	 * default blink modes as 20 Hz, but the registers show the default
+	 * blink mode 0 as 5 Hz. Two identical blink modes aren't very useful,
+	 * so override BMODE_0 here to match the 5Hz "default" described in the
+	 * register map.
+	 */
+	if (priv->properties->arch == SGPIO_ARCH_OCELOT)
+		priv->bmode0 = 2;
+	sgpio_configure_blink_modes(priv);
+
 	return 0;
 }
 
-- 
2.25.1

[RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Colin Foster]

Add additional functions - two blink and two activity, for each SGPIO
output.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/pinctrl/pinctrl-microchip-sgpio.c | 135 +++++++++++++++++++++-
 1 file changed, 130 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c
index 8e081c90bdb2..e3230e5dedc0 100644
--- a/drivers/pinctrl/pinctrl-microchip-sgpio.c
+++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c
@@ -51,6 +51,15 @@ enum {
 	SGPIO_FLAGS_HAS_IRQ	= BIT(0),
 };
 
+enum {
+	FUNC_GPIO,
+	FUNC_BLINK0,
+	FUNC_BLINK1,
+	FUNC_ACTIVITY0,
+	FUNC_ACTIVITY1,
+	FUNC_MAX,
+};
+
 struct sgpio_properties {
 	int arch;
 	int flags;
@@ -60,16 +69,22 @@ struct sgpio_properties {
 #define SGPIO_LUTON_AUTO_REPEAT  BIT(5)
 #define SGPIO_LUTON_PORT_WIDTH   GENMASK(3, 2)
 #define SGPIO_LUTON_CLK_FREQ     GENMASK(11, 0)
+#define SGPIO_LUTON_SIO_BMODE_0	 GENMASK(21, 20)
+#define SGPIO_LUTON_SIO_BMODE_1	 GENMASK(19, 18)
 #define SGPIO_LUTON_BIT_SOURCE   GENMASK(11, 0)
 
 #define SGPIO_OCELOT_AUTO_REPEAT BIT(10)
 #define SGPIO_OCELOT_PORT_WIDTH  GENMASK(8, 7)
 #define SGPIO_OCELOT_CLK_FREQ    GENMASK(19, 8)
+#define SGPIO_OCELOT_SIO_BMODE_0 GENMASK(20, 19)
+#define SGPIO_OCELOT_SIO_BMODE_1 GENMASK(22, 21)
 #define SGPIO_OCELOT_BIT_SOURCE  GENMASK(23, 12)
 
 #define SGPIO_SPARX5_AUTO_REPEAT BIT(6)
 #define SGPIO_SPARX5_PORT_WIDTH  GENMASK(4, 3)
 #define SGPIO_SPARX5_CLK_FREQ    GENMASK(19, 8)
+#define SGPIO_SPARX5_SIO_BMODE_0 GENMASK(16, 15)
+#define SGPIO_SPARX5_SIO_BMODE_1 GENMASK(18, 17)
 #define SGPIO_SPARX5_BIT_SOURCE  GENMASK(23, 12)
 
 #define SGPIO_MASTER_INTR_ENA    BIT(0)
@@ -98,22 +113,46 @@ static const struct sgpio_properties properties_sparx5 = {
 	.regoff = { 0x00, 0x06, 0x26, 0x04, 0x05, 0x2a, 0x32, 0x3a, 0x3e, 0x42 },
 };
 
-static const char * const functions[] = { "gpio" };
+static const char * const function_names[] = {
+	[FUNC_GPIO] = "gpio",
+	[FUNC_BLINK0] = "blink0",
+	[FUNC_BLINK1] = "blink1",
+	[FUNC_ACTIVITY0] = "activity0",
+	[FUNC_ACTIVITY1] = "activity1",
+};
+
+static const int function_values[] = {
+	[FUNC_GPIO] = 0,
+	[FUNC_BLINK0] = 2,
+	[FUNC_BLINK1] = 3,
+	[FUNC_ACTIVITY0] = 4,
+	[FUNC_ACTIVITY1] = 5,
+};
+
+struct sgpio_pmx_func {
+	const char **groups;
+	unsigned int ngroups;
+};
 
 struct sgpio_bank {
 	struct sgpio_priv *priv;
 	bool is_input;
 	struct gpio_chip gpio;
 	struct pinctrl_desc pctl_desc;
+	struct sgpio_pmx_func func[FUNC_MAX];
+	struct pinctrl_pin_desc *pins;
 };
 
 struct sgpio_priv {
 	struct device *dev;
 	struct sgpio_bank in;
 	struct sgpio_bank out;
+	int ngpios;
 	u32 bitcount;
 	u32 ports;
 	u32 clock;
+	u32 bmode0;
+	u32 bmode1;
 	struct regmap *regs;
 	const struct sgpio_properties *properties;
 };
@@ -223,6 +262,32 @@ static inline void sgpio_configure_clock(struct sgpio_priv *priv, u32 clkfrq)
 	sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0, clr, set);
 }
 
+static inline void sgpio_configure_blink_modes(struct sgpio_priv *priv)
+{
+	u32 clr, set;
+
+	switch (priv->properties->arch) {
+	case SGPIO_ARCH_LUTON:
+		clr = SGPIO_LUTON_SIO_BMODE_0 | SGPIO_LUTON_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_LUTON_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_LUTON_SIO_BMODE_1, priv->bmode1);
+		break;
+	case SGPIO_ARCH_OCELOT:
+		clr = SGPIO_OCELOT_SIO_BMODE_0 | SGPIO_OCELOT_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_OCELOT_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_OCELOT_SIO_BMODE_1, priv->bmode1);
+		break;
+	case SGPIO_ARCH_SPARX5:
+		clr = SGPIO_SPARX5_SIO_BMODE_0 | SGPIO_SPARX5_SIO_BMODE_1;
+		set = FIELD_PREP(SGPIO_SPARX5_SIO_BMODE_0, priv->bmode0) |
+		      FIELD_PREP(SGPIO_SPARX5_SIO_BMODE_1, priv->bmode1);
+		break;
+	default:
+		return;
+	}
+	sgpio_clrsetbits(priv, REG_SIO_CONFIG, 0, clr, set);
+}
+
 static void sgpio_output_set(struct sgpio_priv *priv,
 			     struct sgpio_port_addr *addr,
 			     int value)
@@ -352,13 +417,18 @@ static const struct pinconf_ops sgpio_confops = {
 
 static int sgpio_get_functions_count(struct pinctrl_dev *pctldev)
 {
-	return 1;
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+
+	if (bank->is_input)
+		return 1;
+	else
+		return ARRAY_SIZE(function_names);
 }
 
 static const char *sgpio_get_function_name(struct pinctrl_dev *pctldev,
 					   unsigned int function)
 {
-	return functions[0];
+	return function_names[function];
 }
 
 static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
@@ -366,8 +436,10 @@ static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
 				     const char *const **groups,
 				     unsigned *const num_groups)
 {
-	*groups  = functions;
-	*num_groups = ARRAY_SIZE(functions);
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups  = bank->func[function].groups;
+	*num_groups = bank->func[function].ngroups;
 
 	return 0;
 }
@@ -375,6 +447,15 @@ static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
 static int sgpio_pinmux_set_mux(struct pinctrl_dev *pctldev,
 				unsigned int selector, unsigned int group)
 {
+	struct sgpio_bank *bank = pinctrl_dev_get_drvdata(pctldev);
+	struct sgpio_priv *priv = bank->priv;
+	struct sgpio_port_addr addr;
+	int f;
+
+	f = function_values[selector];
+	sgpio_pin_to_addr(priv, group, &addr);
+	sgpio_output_set(priv, &addr, f);
+
 	return 0;
 }
 
@@ -693,6 +774,30 @@ static void sgpio_irq_handler(struct irq_desc *desc)
 	}
 }
 
+static int sgpio_create_group_func_map(struct device *dev,
+				       struct sgpio_bank *bank)
+{
+	struct sgpio_priv *priv = bank->priv;
+	int f, i;
+
+	if (bank->is_input)
+		return 0;
+
+	for (f = 0; f < FUNC_MAX; f++) {
+		bank->func[f].ngroups = priv->ngpios;
+		bank->func[f].groups = devm_kcalloc(dev, priv->ngpios,
+						    sizeof(char *), GFP_KERNEL);
+
+		if (!bank->func[f].groups)
+			return -ENOMEM;
+
+		for (i = 0; i < priv->ngpios; i++)
+			bank->func[f].groups[i] = bank->pins[i].name;
+	}
+
+	return 0;
+}
+
 static int microchip_sgpio_register_bank(struct device *dev,
 					 struct sgpio_priv *priv,
 					 struct fwnode_handle *fwnode,
@@ -716,6 +821,7 @@ static int microchip_sgpio_register_bank(struct device *dev,
 		ngpios = 64;
 	}
 
+	priv->ngpios = ngpios;
 	priv->bitcount = ngpios / SGPIO_BITS_PER_WORD;
 	if (priv->bitcount > SGPIO_MAX_BITS) {
 		dev_err(dev, "Bit width exceeds maximum (%d)\n",
@@ -738,6 +844,7 @@ static int microchip_sgpio_register_bank(struct device *dev,
 
 	pctl_desc->npins = ngpios;
 	pctl_desc->pins = pins;
+	bank->pins = pins;
 
 	for (i = 0; i < ngpios; i++) {
 		struct sgpio_port_addr addr;
@@ -753,6 +860,12 @@ static int microchip_sgpio_register_bank(struct device *dev,
 			return -ENOMEM;
 	}
 
+	ret = sgpio_create_group_func_map(dev, bank);
+	if (ret) {
+		dev_err(dev, "Unable to create group func map.\n");
+		return ret;
+	}
+
 	pctldev = devm_pinctrl_register(dev, pctl_desc, bank);
 	if (IS_ERR(pctldev))
 		return dev_err_probe(dev, PTR_ERR(pctldev), "Failed to register pinctrl\n");
@@ -895,6 +1008,18 @@ static int microchip_sgpio_probe(struct platform_device *pdev)
 		sgpio_writel(priv, 0, REG_PORT_CONFIG, port);
 	sgpio_writel(priv, priv->ports, REG_PORT_ENABLE, 0);
 
+	/*
+	 * The datasheet and register definitions contradict themselves, at
+	 * least for the VSC7512. The Datasheet Revision 4.2 describes both
+	 * default blink modes as 20 Hz, but the registers show the default
+	 * blink mode 0 as 5 Hz. Two identical blink modes aren't very useful,
+	 * so override BMODE_0 here to match the 5Hz "default" described in the
+	 * register map.
+	 */
+	if (priv->properties->arch == SGPIO_ARCH_OCELOT)
+		priv->bmode0 = 2;
+	sgpio_configure_blink_modes(priv);
+
 	return 0;
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Andrew Lunn]

On Wed, Jul 12, 2023 at 03:59:10AM +0200, Christian Marangi wrote:
> On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> > Preface (new for resend):
> > 
> > This is a resend of a patch I'd sent a couple years back. At that time,
> > I was told to wait for hardware-offloaded LEDS. It looks like that time
> > has finally come, so I've changed this from PATCH down to an RFC to make
> > sure this is the right approach for the framework.
> > 
> > Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> > hardware-offloaded LEDs based on network activity. This is currenty
> > managed by way of pinctrl-microchip-sgpio (and this current patch).
> > 
> > The purpose of this resend is two-fold. First, to come up with an idea
> > of how this pinctrl-microchip-sgpio module can fit in with the new
> > hardware-offloaded netdev triggers Christian Marangi recently added. Is
> > this something that should be in the pinctrl module itself? Or should
> > there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> > add?
> >
> 
> I'm a bit out of the loop on what magic OEM did to make LED work on
> ocelot but I feel an ocelot_leds submodule is needed.
> 
> To correctly supports the hw many API needs to be defined and for switch
> I would stick with how things are done with qca8k, codewise and DT wise
> (with how LEDs are defined in DT)
> 
> Ideally the feature for MAC will be generilized and added to the DSA ops
> struct, so having things in the DSA driver would make the migration
> easier.

`ocelot` is a bit of an odd device, since it is both a DSA device for
felix and seville and a pure switchdev device for ocelot.

You need some integration with the switch driver, because i expect
only the switch driver has the knowledge of how LEDs are mapped to
struct netdev and ports. And in order to offload blinking you need
that mapping.

I have some WIP patches to add a generalized DSA interface for LEDs,
and support for mv88e6xxx. I would also like to move qca8k over to
that. So it could be that felix and seville would use that. Ocelot
would need to do it slightly different, but i expect it is just a
layer on top of some shared code, much like the rest of ocelot.

Having pinmux in the middle is interesting. I've no idea how that will
work, but i've not looked at it.

      Andrew

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Andrew Lunn]

On Wed, Jul 12, 2023 at 03:59:10AM +0200, Christian Marangi wrote:
> On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> > Preface (new for resend):
> > 
> > This is a resend of a patch I'd sent a couple years back. At that time,
> > I was told to wait for hardware-offloaded LEDS. It looks like that time
> > has finally come, so I've changed this from PATCH down to an RFC to make
> > sure this is the right approach for the framework.
> > 
> > Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> > hardware-offloaded LEDs based on network activity. This is currenty
> > managed by way of pinctrl-microchip-sgpio (and this current patch).
> > 
> > The purpose of this resend is two-fold. First, to come up with an idea
> > of how this pinctrl-microchip-sgpio module can fit in with the new
> > hardware-offloaded netdev triggers Christian Marangi recently added. Is
> > this something that should be in the pinctrl module itself? Or should
> > there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> > add?
> >
> 
> I'm a bit out of the loop on what magic OEM did to make LED work on
> ocelot but I feel an ocelot_leds submodule is needed.
> 
> To correctly supports the hw many API needs to be defined and for switch
> I would stick with how things are done with qca8k, codewise and DT wise
> (with how LEDs are defined in DT)
> 
> Ideally the feature for MAC will be generilized and added to the DSA ops
> struct, so having things in the DSA driver would make the migration
> easier.

`ocelot` is a bit of an odd device, since it is both a DSA device for
felix and seville and a pure switchdev device for ocelot.

You need some integration with the switch driver, because i expect
only the switch driver has the knowledge of how LEDs are mapped to
struct netdev and ports. And in order to offload blinking you need
that mapping.

I have some WIP patches to add a generalized DSA interface for LEDs,
and support for mv88e6xxx. I would also like to move qca8k over to
that. So it could be that felix and seville would use that. Ocelot
would need to do it slightly different, but i expect it is just a
layer on top of some shared code, much like the rest of ocelot.

Having pinmux in the middle is interesting. I've no idea how that will
work, but i've not looked at it.

      Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Colin Foster]

Hi Andrew and Christian,

On Wed, Jul 12, 2023 at 04:55:44PM +0200, Andrew Lunn wrote:
> On Wed, Jul 12, 2023 at 03:59:10AM +0200, Christian Marangi wrote:
> > On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> > > Preface (new for resend):
> > > 
> > > This is a resend of a patch I'd sent a couple years back. At that time,
> > > I was told to wait for hardware-offloaded LEDS. It looks like that time
> > > has finally come, so I've changed this from PATCH down to an RFC to make
> > > sure this is the right approach for the framework.
> > > 
> > > Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> > > hardware-offloaded LEDs based on network activity. This is currenty
> > > managed by way of pinctrl-microchip-sgpio (and this current patch).
> > > 
> > > The purpose of this resend is two-fold. First, to come up with an idea
> > > of how this pinctrl-microchip-sgpio module can fit in with the new
> > > hardware-offloaded netdev triggers Christian Marangi recently added. Is
> > > this something that should be in the pinctrl module itself? Or should
> > > there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> > > add?
> > >
> > 
> > I'm a bit out of the loop on what magic OEM did to make LED work on
> > ocelot but I feel an ocelot_leds submodule is needed.

The configuration is basically SPI to an GPIO expander. The ocelot chip
fully manages the SPI bus.

> > 
> > To correctly supports the hw many API needs to be defined and for switch
> > I would stick with how things are done with qca8k, codewise and DT wise
> > (with how LEDs are defined in DT)
> > 
> > Ideally the feature for MAC will be generilized and added to the DSA ops
> > struct, so having things in the DSA driver would make the migration
> > easier.
> 
> `ocelot` is a bit of an odd device, since it is both a DSA device for
> felix and seville and a pure switchdev device for ocelot.

Now you tell me :-)

> 
> You need some integration with the switch driver, because i expect
> only the switch driver has the knowledge of how LEDs are mapped to
> struct netdev and ports. And in order to offload blinking you need
> that mapping.
> 
> I have some WIP patches to add a generalized DSA interface for LEDs,
> and support for mv88e6xxx. I would also like to move qca8k over to
> that. So it could be that felix and seville would use that. Ocelot
> would need to do it slightly different, but i expect it is just a
> layer on top of some shared code, much like the rest of ocelot.

Based on this comment, I might sit on the sidelines for a couple more
cycles.

> 
> Having pinmux in the middle is interesting. I've no idea how that will
> work, but i've not looked at it.

Yeah, I'm not exactly sure either. And integration with the switch
driver will be interesting to say the least. I don't know if Felix /
Seville have this capability (probably not).


I'll keep all this on the back of my mind. It seems like there will
definitely be some subtleties in any way. I figure it is better to bring
up the concept earlier than later, so thanks for the feedback!

> 
>       Andrew

Re: [RFC RESEND v1 pinctrl-next 0/1] add blink and activity functions to SGPIO

[Colin Foster]

Hi Andrew and Christian,

On Wed, Jul 12, 2023 at 04:55:44PM +0200, Andrew Lunn wrote:
> On Wed, Jul 12, 2023 at 03:59:10AM +0200, Christian Marangi wrote:
> > On Tue, Jul 11, 2023 at 07:22:49PM -0700, Colin Foster wrote:
> > > Preface (new for resend):
> > > 
> > > This is a resend of a patch I'd sent a couple years back. At that time,
> > > I was told to wait for hardware-offloaded LEDS. It looks like that time
> > > has finally come, so I've changed this from PATCH down to an RFC to make
> > > sure this is the right approach for the framework.
> > > 
> > > Ocelot chips (VSC7511, VSC7512, VSC7513, VSC7514) have support for
> > > hardware-offloaded LEDs based on network activity. This is currenty
> > > managed by way of pinctrl-microchip-sgpio (and this current patch).
> > > 
> > > The purpose of this resend is two-fold. First, to come up with an idea
> > > of how this pinctrl-microchip-sgpio module can fit in with the new
> > > hardware-offloaded netdev triggers Christian Marangi recently added. Is
> > > this something that should be in the pinctrl module itself? Or should
> > > there be a drivers/net/ethernet/mscc/ocelot_leds.c module that I should
> > > add?
> > >
> > 
> > I'm a bit out of the loop on what magic OEM did to make LED work on
> > ocelot but I feel an ocelot_leds submodule is needed.

The configuration is basically SPI to an GPIO expander. The ocelot chip
fully manages the SPI bus.

> > 
> > To correctly supports the hw many API needs to be defined and for switch
> > I would stick with how things are done with qca8k, codewise and DT wise
> > (with how LEDs are defined in DT)
> > 
> > Ideally the feature for MAC will be generilized and added to the DSA ops
> > struct, so having things in the DSA driver would make the migration
> > easier.
> 
> `ocelot` is a bit of an odd device, since it is both a DSA device for
> felix and seville and a pure switchdev device for ocelot.

Now you tell me :-)

> 
> You need some integration with the switch driver, because i expect
> only the switch driver has the knowledge of how LEDs are mapped to
> struct netdev and ports. And in order to offload blinking you need
> that mapping.
> 
> I have some WIP patches to add a generalized DSA interface for LEDs,
> and support for mv88e6xxx. I would also like to move qca8k over to
> that. So it could be that felix and seville would use that. Ocelot
> would need to do it slightly different, but i expect it is just a
> layer on top of some shared code, much like the rest of ocelot.

Based on this comment, I might sit on the sidelines for a couple more
cycles.

> 
> Having pinmux in the middle is interesting. I've no idea how that will
> work, but i've not looked at it.

Yeah, I'm not exactly sure either. And integration with the switch
driver will be interesting to say the least. I don't know if Felix /
Seville have this capability (probably not).


I'll keep all this on the back of my mind. It seems like there will
definitely be some subtleties in any way. I figure it is better to bring
up the concept earlier than later, so thanks for the feedback!

> 
>       Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Linus Walleij]

On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
<colin.foster@in-advantage.com> wrote:

> Add additional functions - two blink and two activity, for each SGPIO
> output.
>
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>

Could Lars or Horatiu review this patch? You guys know the driver
best.

Yours,
Linus Walleij

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Linus Walleij]

On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
<colin.foster@in-advantage.com> wrote:

> Add additional functions - two blink and two activity, for each SGPIO
> output.
>
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>

Could Lars or Horatiu review this patch? You guys know the driver
best.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Colin Foster]

On Thu, Jul 20, 2023 at 09:25:32PM +0200, Linus Walleij wrote:
> On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
> <colin.foster@in-advantage.com> wrote:
> 
> > Add additional functions - two blink and two activity, for each SGPIO
> > output.
> >
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> 
> Could Lars or Horatiu review this patch? You guys know the driver
> best.

Agreed. Please don't merge this without their approval and hopefully
testing.

I did demote this patch I've been dragging around since 2021 to RFC
status because I'm more interested in making sure it will fit in with
the work on hardware-offloaded network activity LED work that's being
done. I took Andrew's response to the cover letter as an suggestion to
hold off for a little while longer. I can be patient.

Also, this RFC was two-fold. I don't want to duplicate efforts, and I
know this pinctrl driver was written with this functionality in mind. If
someone out there has a hankering to get those LEDs blinking and they
don't want to wait around for me, feel free to use this as a starting
point. I might not get around to the whole netdev trigger thing for
quite some time!


Colin Foster

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Colin Foster]

On Thu, Jul 20, 2023 at 09:25:32PM +0200, Linus Walleij wrote:
> On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
> <colin.foster@in-advantage.com> wrote:
> 
> > Add additional functions - two blink and two activity, for each SGPIO
> > output.
> >
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> 
> Could Lars or Horatiu review this patch? You guys know the driver
> best.

Agreed. Please don't merge this without their approval and hopefully
testing.

I did demote this patch I've been dragging around since 2021 to RFC
status because I'm more interested in making sure it will fit in with
the work on hardware-offloaded network activity LED work that's being
done. I took Andrew's response to the cover letter as an suggestion to
hold off for a little while longer. I can be patient.

Also, this RFC was two-fold. I don't want to duplicate efforts, and I
know this pinctrl driver was written with this functionality in mind. If
someone out there has a hankering to get those LEDs blinking and they
don't want to wait around for me, feel free to use this as a starting
point. I might not get around to the whole netdev trigger thing for
quite some time!


Colin Foster

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Horatiu Vultur]

The 07/20/2023 14:02, Colin Foster wrote:

Hi,

> 
> On Thu, Jul 20, 2023 at 09:25:32PM +0200, Linus Walleij wrote:
> > On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
> > <colin.foster@in-advantage.com> wrote:
> >
> > > Add additional functions - two blink and two activity, for each SGPIO
> > > output.
> > >
> > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> >
> > Could Lars or Horatiu review this patch? You guys know the driver
> > best.
> 
> Agreed. Please don't merge this without their approval and hopefully
> testing.
> 

I have tried to apply the patch to test it, but unfortunately it doesn't
apply.
I have looked through the changes and they seem OK.

> I did demote this patch I've been dragging around since 2021 to RFC
> status because I'm more interested in making sure it will fit in with
> the work on hardware-offloaded network activity LED work that's being
> done. I took Andrew's response to the cover letter as an suggestion to
> hold off for a little while longer. I can be patient.
> 
> Also, this RFC was two-fold. I don't want to duplicate efforts, and I
> know this pinctrl driver was written with this functionality in mind. If
> someone out there has a hankering to get those LEDs blinking and they
> don't want to wait around for me, feel free to use this as a starting
> point. I might not get around to the whole netdev trigger thing for
> quite some time!
> 
> 
> Colin Foster

-- 
/Horatiu

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Colin Foster]

Hi Horaitu,

On Mon, Jul 24, 2023 at 08:59:57AM +0200, Horatiu Vultur wrote:
> The 07/20/2023 14:02, Colin Foster wrote:
> 
> Hi,
> 
> > 
> > On Thu, Jul 20, 2023 at 09:25:32PM +0200, Linus Walleij wrote:
> > > On Wed, Jul 12, 2023 at 4:23 AM Colin Foster
> > > <colin.foster@in-advantage.com> wrote:
> > >
> > > > Add additional functions - two blink and two activity, for each SGPIO
> > > > output.
> > > >
> > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > >
> > > Could Lars or Horatiu review this patch? You guys know the driver
> > > best.
> > 
> > Agreed. Please don't merge this without their approval and hopefully
> > testing.
> > 
> 
> I have tried to apply the patch to test it, but unfortunately it doesn't
> apply.
> I have looked through the changes and they seem OK.

Is there a tree I should test these patches against? I don't have any
active development going on so I've been hopping along the latest RCs
instead of keeping up with net-next, gpio-next, or otherwise...

Anyway, thanks for taking a look!

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Linus Walleij]

On Mon, Jul 24, 2023 at 8:55 PM Colin Foster
<colin.foster@in-advantage.com> wrote:

> Is there a tree I should test these patches against? I don't have any
> active development going on so I've been hopping along the latest RCs
> instead of keeping up with net-next, gpio-next, or otherwise...

Use the "devel" branch in the pinctrl tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

If you resend based on this branch I'll apply the patch, I think Horatiu's
reply counts as an Acked-by so record it as such.

Yours,
Linus Walleij

Re: [RFC RESEND v1 pinctrl-next 1/1] pinctrl: microchip-sgpio: add activity and blink functionality

[Linus Walleij]

On Mon, Jul 24, 2023 at 8:55 PM Colin Foster
<colin.foster@in-advantage.com> wrote:

> Is there a tree I should test these patches against? I don't have any
> active development going on so I've been hopping along the latest RCs
> instead of keeping up with net-next, gpio-next, or otherwise...

Use the "devel" branch in the pinctrl tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

If you resend based on this branch I'll apply the patch, I think Horatiu's
reply counts as an Acked-by so record it as such.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel