Re: [PATCH v8 1/2] led: led_cortina: Add CAxxx LED support
Hi Alex,
On Thu, 21 May 2020 at 18:43, Alex Nemirovsky
wrote:
>
> From: Jway Lin
>
> Add Cortina Access LED controller support for CA SOCs
>
> Signed-off-by: Jway Lin
> Signed-off-by: Alex Nemirovsky
> CC: Simon Glass
>
> ---
>
> Changes in v8:
> - No code change
> - Split out individual driver from Cortina Package 2 patch series
> to help streamline acceptence into master
>
> Changes in v7:
> - rename OFFSET to SHIFT from macros
> - add additinal struct comments
> - Reading the DT should really happen in the ofdata_to_platdata method
>
> Changes in v4:
> - remove unused macros
> - remove cortina prefix from macros
> - remove use BSS variable
> - further cleanup to meet code style guidelines
> - add additinal struct comments
> - rename DT blink rate symbol
>
> MAINTAINERS | 8 +-
> drivers/led/Kconfig | 8 ++
> drivers/led/Makefile | 1 +
> drivers/led/led_cortina.c | 305
> ++
> 4 files changed, 321 insertions(+), 1 deletion(-)
> create mode 100644 drivers/led/led_cortina.c
Reviewed-by: Simon Glass
One comment below
[..]
> diff --git a/drivers/led/led_cortina.c b/drivers/led/led_cortina.c
> new file mode 100644
> index 000..a6d9159
> --- /dev/null
> +++ b/drivers/led/led_cortina.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
[..]
> +static int cortina_led_set_state(struct udevice *dev, enum led_state_t state)
> +{
> + u32 val;
> + struct cortina_led_cfg *priv = dev_get_priv(dev);
> +
> + val = readl(priv->regs);
> + val &= ~(LED_OFF_ON_MASK << LED_OFF_ON_SHIFT);
> +
> + switch (state) {
> + case LEDST_OFF:
> + val &= ~LED_SW_EVENT;
> + val |= CA_LED_OFF << LED_OFF_ON_SHIFT;
> + cortina_led_write(priv->regs, val);
> + break;
> + case LEDST_ON:
> + val |= LED_SW_EVENT;
> + val |= CA_LED_ON << LED_OFF_ON_SHIFT;
> + cortina_led_write(priv->regs, val);
> + break;
> + case LEDST_TOGGLE:
> + if (cortina_led_get_state(dev) == LEDST_OFF)
> + return cortina_led_set_state(dev, LEDST_ON);
> + else
> + return cortina_led_set_state(dev, LEDST_OFF);
> + break;
> +#ifdef CONFIG_LED_BLINK
> + case LEDST_BLINK:
> + val &= ~LED_SW_EVENT;
> + val |= CA_LED_OFF << LED_OFF_ON_SHIFT;
> + val |= LED_EVENT_BLINK_MASK << LED_EVENT_BLINK_SHIFT;
> + cortina_led_write(priv->regs, val);
> + break;
> +#endif
Try to void #ifdef. You should be able to do this:
case CONFIG_LED_BLINK:
if (IS_ENABLED(CONFIG_LED_BLINK)) {
val code
...
break;
}
/* no break */
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct led_ops cortina_led_ops = {
> + .get_state = cortina_led_get_state,
> + .set_state = cortina_led_set_state,
> +};
> +
> +static int ca_led_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct led_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +
> + /* Top-level LED node */
> + if (!uc_plat->label) {
> + struct cortina_led_platdata *plt = dev_get_platdata(dev);
> +
> + plt->rate1 =
> + dev_read_u32_default(dev, "Cortina,blink-rate1", 256);
> + plt->rate2 =
> + dev_read_u32_default(dev, "Cortina,blink-rate2", 512);
> + plt->ctrl_regs = dev_remap_addr(dev);
> + } else {
> + struct cortina_led_cfg *priv = dev_get_priv(dev);
> +
> + priv->regs = dev_remap_addr(dev_get_parent(dev));
> + priv->pin = dev_read_u32_default(dev, "pin", LED_MAX_COUNT);
> + priv->blink_sel = dev_read_u32_default(dev, "blink-sel", 0);
> + priv->off_event = dev_read_u32_default(dev, "off-event", 0);
> + priv->blink_event = dev_read_u32_default(dev, "blink-event",
> 0);
> + priv->on_event = dev_read_u32_default(dev, "on-event", 0);
> + priv->port = dev_read_u32_default(dev, "port", 0);
> +
> + if (dev_read_bool(dev, "active-low"))
> + priv->active_low = true;
> + else
> + priv->active_low = false;
> + }
> +
> + return 0;
> +}
> +
> +static int cortina_led_probe(struct udevice *dev)
> +{
> + struct led_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +
> + /* Top-level LED node */
> + if (!uc_plat->label) {
> + struct cortina_led_platdata *platdata = dev_get_platdata(dev);
> + u32 reg_value, val;
> + u16 rate1, rate2;
> +
> + if (!platdata->ctrl_regs)
> + return -EINVAL;
> +
> + reg_value =
[PATCH v8 1/2] led: led_cortina: Add CAxxx LED support
From: Jway Lin
Add Cortina Access LED controller support for CA SOCs
Signed-off-by: Jway Lin
Signed-off-by: Alex Nemirovsky
CC: Simon Glass
---
Changes in v8:
- No code change
- Split out individual driver from Cortina Package 2 patch series
to help streamline acceptence into master
Changes in v7:
- rename OFFSET to SHIFT from macros
- add additinal struct comments
- Reading the DT should really happen in the ofdata_to_platdata method
Changes in v4:
- remove unused macros
- remove cortina prefix from macros
- remove use BSS variable
- further cleanup to meet code style guidelines
- add additinal struct comments
- rename DT blink rate symbol
MAINTAINERS | 8 +-
drivers/led/Kconfig | 8 ++
drivers/led/Makefile | 1 +
drivers/led/led_cortina.c | 305 ++
4 files changed, 321 insertions(+), 1 deletion(-)
create mode 100644 drivers/led/led_cortina.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8add9d4..7a624bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -181,6 +181,9 @@ F: drivers/gpio/cortina_gpio.c
F: drivers/watchdog/cortina_wdt.c
F: drivers/serial/serial_cortina.c
F: drivers/mmc/ca_dw_mmc.c
+F: drivers/i2c/i2c-cortina.c
+F: drivers/i2c/i2c-cortina.h
+F: drivers/led/led_cortina.c
ARM/CZ.NIC TURRIS MOX SUPPORT
M: Marek Behun
@@ -732,6 +735,9 @@ F: drivers/gpio/cortina_gpio.c
F: drivers/watchdog/cortina_wdt.c
F: drivers/serial/serial_cortina.c
F: drivers/mmc/ca_dw_mmc.c
+F: drivers/i2c/i2c-cortina.c
+F: drivers/i2c/i2c-cortina.h
+F: drivers/led/led_cortina.c
MIPS MSCC
M: Gregory CLEMENT
@@ -824,7 +830,7 @@ S: Maintained
F: arch/powerpc/
POWERPC MPC8XX
-M: Christophe Leroy
+M: Christophe Leroy
S: Maintained
T: git https://gitlab.denx.de/u-boot/custodians/u-boot-mpc8xx.git
F: arch/powerpc/cpu/mpc8xx/
diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index 6675934..cc87fbf 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -35,6 +35,14 @@ config LED_BCM6858
This option enables support for LEDs connected to the BCM6858
HW has blinking capabilities and up to 32 LEDs can be controlled.
+config LED_CORTINA
+ bool "LED Support for Cortina Access CA SoCs"
+ depends on LED && (CORTINA_PLATFORM)
+ help
+ This option enables support for LEDs connected to the Cortina
+ Access CA SOCs.
+
+
config LED_BLINK
bool "Support LED blinking"
depends on LED
diff --git a/drivers/led/Makefile b/drivers/led/Makefile
index 3654dd3..8e3ae7f 100644
--- a/drivers/led/Makefile
+++ b/drivers/led/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_LED_BCM6328) += led_bcm6328.o
obj-$(CONFIG_LED_BCM6358) += led_bcm6358.o
obj-$(CONFIG_LED_BCM6858) += led_bcm6858.o
obj-$(CONFIG_$(SPL_)LED_GPIO) += led_gpio.o
+obj-$(CONFIG_LED_CORTINA) += led_cortina.o
diff --git a/drivers/led/led_cortina.c b/drivers/led/led_cortina.c
new file mode 100644
index 000..a6d9159
--- /dev/null
+++ b/drivers/led/led_cortina.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright (C) 2020 Cortina-Access
+ * Author: Jway Lin
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LED_MAX_HW_BLINK 127
+#define LED_MAX_COUNT 16
+
+/* LED_CONTROL fields */
+#define LED_BLINK_RATE1_SHIFT 0
+#define LED_BLINK_RATE1_MASK 0xff
+#define LED_BLINK_RATE2_SHIFT 8
+#define LED_BLINK_RATE2_MASK 0xff
+#define LED_CLK_TEST BIT(16)
+#define LED_CLK_POLARITY BIT(17)
+#define LED_CLK_TEST_MODE BIT(16)
+#define LED_CLK_TEST_RX_TEST BIT(30)
+#define LED_CLK_TEST_TX_TEST BIT(31)
+
+/* LED_CONFIG fields */
+#define LED_EVENT_ON_SHIFT 0
+#define LED_EVENT_ON_MASK 0x7
+#define LED_EVENT_BLINK_SHIFT 3
+#define LED_EVENT_BLINK_MASK 0x7
+#define LED_EVENT_OFF_SHIFT6
+#define LED_EVENT_OFF_MASK 0x7
+#define LED_OFF_ON_SHIFT 9
+#define LED_OFF_ON_MASK0x3
+#define LED_PORT_SHIFT 11
+#define LED_PORT_MASK 0x7
+#define LED_OFF_VALBIT(14)
+#define LED_SW_EVENT BIT(15)
+#define LED_BLINK_SEL BIT(16)
+
+/* LED_CONFIG structures */
+struct cortina_led_cfg {
+ void __iomem *regs;
+ u32 pin;/* LED pin nubmer */
+ bool active_low;/*Active-High or Active-Low*/
+ u32 off_event; /* set led off event (RX,TX,SW)*/
+ u32 blink_event;/* set led blink event (RX,TX,SW)*/
+ u32 on_event; /* set led on event (RX,TX,SW)*/
+ u32 port; /* corresponding ethernet port */
+ int blink_sel; /* select blink-rate1 or blink-rate2 */
+};
+
+/* LED_control structures */
+struct cortina_led_platdata {
+ void
