MT7530's LED controller can be used as GPIO controller. Add support for
it.
DENG Qingfang (2):
dt-bindings: net: dsa: add MT7530 GPIO controller binding
drivers: net: dsa: mt7530: MT7530 optional GPIO support
.../devicetree/bindings/net/dsa/mt7530.txt | 6 ++
drivers/net/dsa/mt7530.c | 96 +++++++++++++++++++
drivers/net/dsa/mt7530.h | 20 ++++
3 files changed, 122 insertions(+)
--
2.25.1
Add device tree binding to support MT7530 GPIO controller.
Signed-off-by: DENG Qingfang <[email protected]>
---
Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
index 560369efad6c..de04626a8e9d 100644
--- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -76,6 +76,12 @@ phy-mode must be set, see also example 2 below!
* mt7621: phy-mode = "rgmii-txid";
* mt7623: phy-mode = "rgmii";
+Optional properties:
+
+- gpio-controller: Boolean; if defined, MT7530's LED controller will run on
+ GPIO mode.
+- #gpio-cells: Must be 2 if gpio-controller is defined.
+
See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional
required, optional properties and how the integrated switch subnodes must
be specified.
--
2.25.1
MT7530's LED controller can drive up to 15 LED/GPIOs.
Add support for GPIO control and allow users to use its GPIOs by
setting gpio-controller property in device tree.
Signed-off-by: DENG Qingfang <[email protected]>
---
drivers/net/dsa/mt7530.c | 96 ++++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mt7530.h | 20 +++++++++
2 files changed, 116 insertions(+)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a67cac15a724..0686d8cbd086 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -18,6 +18,7 @@
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
#include <net/dsa.h>
#include "mt7530.h"
@@ -1639,6 +1640,95 @@ mtk_get_tag_protocol(struct dsa_switch *ds, int port,
}
}
+static u32
+mt7530_gpio_to_bit(unsigned int offset)
+{
+ return BIT(offset + offset / 3);
+}
+
+static int
+mt7530_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct mt7530_priv *priv = gpiochip_get_data(gc);
+ u32 bit = mt7530_gpio_to_bit(offset);
+
+ return !!(mt7530_read(priv, MT7530_LED_GPIO_DATA) & bit);
+}
+
+static void
+mt7530_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+ struct mt7530_priv *priv = gpiochip_get_data(gc);
+ u32 bit = mt7530_gpio_to_bit(offset);
+
+ if (value)
+ mt7530_set(priv, MT7530_LED_GPIO_DATA, bit);
+ else
+ mt7530_clear(priv, MT7530_LED_GPIO_DATA, bit);
+}
+
+static int
+mt7530_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+ struct mt7530_priv *priv = gpiochip_get_data(gc);
+ u32 bit = mt7530_gpio_to_bit(offset);
+
+ return (mt7530_read(priv, MT7530_LED_GPIO_DIR) & bit) ?
+ GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
+}
+
+static int
+mt7530_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+ struct mt7530_priv *priv = gpiochip_get_data(gc);
+ u32 bit = mt7530_gpio_to_bit(offset);
+
+ mt7530_clear(priv, MT7530_LED_GPIO_DIR, bit);
+ mt7530_clear(priv, MT7530_LED_GPIO_OE, bit);
+
+ return 0;
+}
+
+static int
+mt7530_gpio_direction_output(struct gpio_chip *gc, unsigned int offset, int value)
+{
+ struct mt7530_priv *priv = gpiochip_get_data(gc);
+ u32 bit = mt7530_gpio_to_bit(offset);
+
+ mt7530_set(priv, MT7530_LED_GPIO_DIR, bit);
+ mt7530_set(priv, MT7530_LED_GPIO_OE, bit);
+ mt7530_gpio_set(gc, offset, value);
+
+ return 0;
+}
+
+static int
+mt7530_setup_gpio(struct mt7530_priv *priv)
+{
+ struct device *dev = priv->dev;
+ struct gpio_chip *gc;
+
+ gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
+ if (!gc)
+ return -ENOMEM;
+
+ mt7530_write(priv, MT7530_LED_IO_MODE, 0);
+
+ gc->label = "mt7530";
+ gc->parent = dev;
+ gc->owner = THIS_MODULE;
+ gc->get_direction = mt7530_gpio_get_direction;
+ gc->direction_input = mt7530_gpio_direction_input;
+ gc->direction_output = mt7530_gpio_direction_output;
+ gc->get = mt7530_gpio_get;
+ gc->set = mt7530_gpio_set;
+ gc->base = -1;
+ gc->ngpio = 15;
+ gc->can_sleep = true;
+
+ return devm_gpiochip_add_data(dev, gc, priv);
+}
+
static int
mt7530_setup(struct dsa_switch *ds)
{
@@ -1781,6 +1871,12 @@ mt7530_setup(struct dsa_switch *ds)
}
}
+ if (of_property_read_bool(priv->dev->of_node, "gpio-controller")) {
+ ret = mt7530_setup_gpio(priv);
+ if (ret)
+ return ret;
+ }
+
mt7530_setup_port5(ds, interface);
/* Flush the FDB table */
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 32d8969b3ace..e7903ecc6a7c 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -554,6 +554,26 @@ enum mt7531_clk_skew {
#define MT7531_GPIO12_RG_RXD3_MASK GENMASK(19, 16)
#define MT7531_EXT_P_MDIO_12 (2 << 16)
+/* Registers for LED GPIO control (MT7530 only)
+ * All registers follow this pattern:
+ * [2:0] port 0
+ * [6:4] port 1
+ * [10:8] port 2
+ * [14:12] port 3
+ * [18:16] port 4
+ */
+
+/* LED enable, 0: Disable, 1: Enable (Default) */
+#define MT7530_LED_EN 0x7d00
+/* LED mode, 0: GPIO mode, 1: PHY mode (Default) */
+#define MT7530_LED_IO_MODE 0x7d04
+/* GPIO direction, 0: Input, 1: Output */
+#define MT7530_LED_GPIO_DIR 0x7d10
+/* GPIO output enable, 0: Disable, 1: Enable */
+#define MT7530_LED_GPIO_OE 0x7d14
+/* GPIO value, 0: Low, 1: High */
+#define MT7530_LED_GPIO_DATA 0x7d18
+
#define MT7530_CREV 0x7ffc
#define CHIP_NAME_SHIFT 16
#define MT7530_ID 0x7530
--
2.25.1
On Mon, Jan 11, 2021 at 01:44:28PM +0800, DENG Qingfang wrote:
> +static int
> +mt7530_gpio_direction_output(struct gpio_chip *gc, unsigned int offset, int value)
> +{
> + struct mt7530_priv *priv = gpiochip_get_data(gc);
> + u32 bit = mt7530_gpio_to_bit(offset);
> +
> + mt7530_set(priv, MT7530_LED_GPIO_DIR, bit);
> + mt7530_set(priv, MT7530_LED_GPIO_OE, bit);
> + mt7530_gpio_set(gc, offset, value);
FYI, Documentation/driver-api/gpio/consumer.rst says:
For output GPIOs, the value provided becomes the initial output value.
This helps avoid signal glitching during system startup.
Setting the pin to be an output, and then setting its initial value
does not avoid the glitch. You may wish to investigate whether you
can set the value before setting the pin as an output to avoid this
issue.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Jan 11, 2021 at 7:04 PM Russell King - ARM Linux admin
<[email protected]> wrote:
>
> FYI, Documentation/driver-api/gpio/consumer.rst says:
>
> For output GPIOs, the value provided becomes the initial output value.
> This helps avoid signal glitching during system startup.
>
> Setting the pin to be an output, and then setting its initial value
> does not avoid the glitch. You may wish to investigate whether you
> can set the value before setting the pin as an output to avoid this
> issue.
>
So, setting the Output Enable bit _after_ setting the direction and
initial value should avoid this issue. Right?
On Mon, Jan 11, 2021 at 01:44:26PM +0800, DENG Qingfang wrote:
> MT7530's LED controller can be used as GPIO controller. Add support for
> it.
>
> DENG Qingfang (2):
> dt-bindings: net: dsa: add MT7530 GPIO controller binding
> drivers: net: dsa: mt7530: MT7530 optional GPIO support
>
> .../devicetree/bindings/net/dsa/mt7530.txt | 6 ++
> drivers/net/dsa/mt7530.c | 96 +++++++++++++++++++
> drivers/net/dsa/mt7530.h | 20 ++++
> 3 files changed, 122 insertions(+)
>
> --
> 2.25.1
Adding GPIO and LED maintainers to also have a look.
https://patchwork.kernel.org/project/netdevbpf/cover/[email protected]/
On Mon, Jan 11, 2021 at 09:40:00PM +0800, DENG Qingfang wrote:
> On Mon, Jan 11, 2021 at 7:04 PM Russell King - ARM Linux admin
> <[email protected]> wrote:
> >
> > FYI, Documentation/driver-api/gpio/consumer.rst says:
> >
> > For output GPIOs, the value provided becomes the initial output value.
> > This helps avoid signal glitching during system startup.
> >
> > Setting the pin to be an output, and then setting its initial value
> > does not avoid the glitch. You may wish to investigate whether you
> > can set the value before setting the pin as an output to avoid this
> > issue.
> >
>
> So, setting the Output Enable bit _after_ setting the direction and
> initial value should avoid this issue. Right?
It depends on the hardware. I don't know how your hardware works, so
I can't say whether doing anything will result in correct behaviour,
or even work.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Qingfang,
what modes does the LED support? Does it support blinking on rx/tx?
What about link status?
I'd like to know because I am still working on patches which add
ethernet PHY/switch LEDs, with transparent offloading of netdev trigger.
Marek
On Mon, 11 Jan 2021 13:44:26 +0800
DENG Qingfang <[email protected]> wrote:
> MT7530's LED controller can be used as GPIO controller. Add support for
> it.
>
> DENG Qingfang (2):
> dt-bindings: net: dsa: add MT7530 GPIO controller binding
> drivers: net: dsa: mt7530: MT7530 optional GPIO support
>
> .../devicetree/bindings/net/dsa/mt7530.txt | 6 ++
> drivers/net/dsa/mt7530.c | 96 +++++++++++++++++++
> drivers/net/dsa/mt7530.h | 20 ++++
> 3 files changed, 122 insertions(+)
>
Hi Marek,
On Mon, Jan 11, 2021 at 11:46 PM Marek Behún <[email protected]> wrote:
>
> what modes does the LED support? Does it support blinking on rx/tx?
> What about link status?
Yes. But unfortunately they cannot be controlled individually, unless
on GPIO mode.
> I'd like to know because I am still working on patches which add
> ethernet PHY/switch LEDs, with transparent offloading of netdev trigger.
>
> Marek
On Mon, Jan 11, 2021 at 2:43 PM Vladimir Oltean <[email protected]> wrote:
>
> On Mon, Jan 11, 2021 at 01:44:26PM +0800, DENG Qingfang wrote:
> > MT7530's LED controller can be used as GPIO controller. Add support for
> > it.
> >
> > DENG Qingfang (2):
> > dt-bindings: net: dsa: add MT7530 GPIO controller binding
> > drivers: net: dsa: mt7530: MT7530 optional GPIO support
> >
> > .../devicetree/bindings/net/dsa/mt7530.txt | 6 ++
> > drivers/net/dsa/mt7530.c | 96 +++++++++++++++++++
> > drivers/net/dsa/mt7530.h | 20 ++++
> > 3 files changed, 122 insertions(+)
> >
> > --
> > 2.25.1
>
> Adding GPIO and LED maintainers to also have a look.
> https://patchwork.kernel.org/project/netdevbpf/cover/[email protected]/
Can you resend the series with GPIO maintainers in CC?
Bart
On 1/11/21 6:50 PM, DENG Qingfang wrote:
> Hi Marek,
>
> On Mon, Jan 11, 2021 at 11:46 PM Marek Behún <[email protected]> wrote:
>>
>> what modes does the LED support? Does it support blinking on rx/tx?
>> What about link status?
Just to be crystal clear here, if you configure the LEDs to be in GPIO
mode, you can defer to the leds-gpio driver for all configuration, and
you can still offload blinking of the LEDs to the hardware or does
blinking require you to use a software managed timer?
--
Florian
On Mon, 11 Jan 2021 13:44:27 +0800, DENG Qingfang wrote:
> Add device tree binding to support MT7530 GPIO controller.
>
> Signed-off-by: DENG Qingfang <[email protected]>
> ---
> Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Mon, Jan 11, 2021 at 6:46 AM DENG Qingfang <[email protected]> wrote:
> MT7530's LED controller can drive up to 15 LED/GPIOs.
>
> Add support for GPIO control and allow users to use its GPIOs by
> setting gpio-controller property in device tree.
>
> Signed-off-by: DENG Qingfang <[email protected]>
Double-check the initial output conditions as indicated by
Russell, if you really want to be thorough, use an oscilloscope
but check the specs at least.
> +static u32
> +mt7530_gpio_to_bit(unsigned int offset)
> +{
> + return BIT(offset + offset / 3);
> +}
So for offset 0..14 this becomes bits
0, 1, 2, 4, 5, 6, 8, 9, 10, 12 ... 18
What is the logic in this and is it what you intend?
Please add a comment explaining what the offset is supposed
to become for offsets 0..14 and why.
> + gc->ngpio = 15;
And it really IS 15 not 16? Not that I know network equipment
very well...
Yours,
Linus Walleij
On Mon, Jan 11, 2021 at 6:46 AM DENG Qingfang <[email protected]> wrote:
> Add device tree binding to support MT7530 GPIO controller.
>
> Signed-off-by: DENG Qingfang <[email protected]>
This is a good way to turn it into a GPIO controller.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
Hi Linus,
On Mon, Jan 18, 2021 at 10:55 PM Linus Walleij <[email protected]> wrote:
>
> So for offset 0..14 this becomes bits
> 0, 1, 2, 4, 5, 6, 8, 9, 10, 12 ... 18
>
> What is the logic in this and is it what you intend?
Yes. Bit 0..2 are phy 0's LED 0..2, bit 4..6 are phy 1's LED 0..2, etc.
> Please add a comment explaining what the offset is supposed
> to become for offsets 0..14 and why.
I already added to mt7530.h, perhaps I should copy it here?
>
> > + gc->ngpio = 15;
>
> And it really IS 15 not 16? Not that I know network equipment
> very well...
Yes, 3 LEDs for each phy.
>
> Yours,
> Linus Walleij
On Tue, Jan 19, 2021 at 4:20 AM DENG Qingfang <[email protected]> wrote:
> On Mon, Jan 18, 2021 at 10:55 PM Linus Walleij <[email protected]> wrote:
> >
> > So for offset 0..14 this becomes bits
> > 0, 1, 2, 4, 5, 6, 8, 9, 10, 12 ... 18
> >
> > What is the logic in this and is it what you intend?
>
> Yes. Bit 0..2 are phy 0's LED 0..2, bit 4..6 are phy 1's LED 0..2, etc.
OK add a comment and explain how the bits relate
to each PHY and how the lines are arranged per-phy
so it is crystal clear for people reading the driver.
Thanks!
Linus Walleij