Hi, On 9/3/21 5:36 AM, Maxime Ripard wrote: > On Thu, Sep 02, 2021 at 06:42:28PM -0500, Samuel Holland wrote: >> Some Allwinner sunxi SoCs, starting with the R329, contain an LED >> controller designed to drive RGB LED pixels. Add a driver for it using >> the multicolor LED framework, and with LEDs defined in the device tree. >> >> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx> >> --- >> drivers/leds/Kconfig | 8 + >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-sunxi.c | 562 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 571 insertions(+) >> create mode 100644 drivers/leds/leds-sunxi.c >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index ed800f5da7d8..559d2ca0a7f4 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -297,6 +297,14 @@ config LEDS_SUNFIRE >> This option enables support for the Left, Middle, and Right >> LEDs on the I/O and CPU boards of SunFire UltraSPARC servers. >> >> +config LEDS_SUNXI >> + tristate "LED support for Allwinner sunxi LED controller" >> + depends on LEDS_CLASS >> + depends on ARCH_SUNXI || COMPILE_TEST >> + help >> + This option enables support for the LED controller provided in >> + some Allwinner sunxi SoCs. >> + > > Same comment for the name Are you concerned about the help text only, or do you also want me to rename the Kconfig symbol? I am happy to change the help text to something like: "This option enables support for the LED controller provided in the Allwinner R329 and D1 SoCs." But I don't know of any satisfying way to rename the Kconfig symbol. There is no general category name for "R329 and D1." >> config LEDS_IPAQ_MICRO >> tristate "LED Support for the Compaq iPAQ h3xxx" >> depends on LEDS_CLASS >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile >> index c636ec069612..3395f188e5bf 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -77,6 +77,7 @@ obj-$(CONFIG_LEDS_PWM) += leds-pwm.o >> obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o >> obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o >> obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o >> +obj-$(CONFIG_LEDS_SUNXI) += leds-sunxi.o >> obj-$(CONFIG_LEDS_SUNFIRE) += leds-sunfire.o >> obj-$(CONFIG_LEDS_SYSCON) += leds-syscon.o >> obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o >> diff --git a/drivers/leds/leds-sunxi.c b/drivers/leds/leds-sunxi.c >> new file mode 100644 >> index 000000000000..6569d64e0c76 >> --- /dev/null >> +++ b/drivers/leds/leds-sunxi.c >> @@ -0,0 +1,562 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// >> +// Copyright (c) 2021 Samuel Holland <samuel@xxxxxxxxxxxx> >> +// >> +// Partly based on drivers/leds/leds-turris-omnia.c, which is: >> +// Copyright (c) 2020 by Marek Behún <kabel@xxxxxxxxxx> >> +// >> + >> +#include <linux/clk.h> >> +#include <linux/dma-mapping.h> >> +#include <linux/dmaengine.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/led-class-multicolor.h> >> +#include <linux/leds.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/reset.h> >> +#include <linux/spinlock.h> >> + >> +#define LEDC_CTRL_REG 0x0000 >> +#define LEDC_CTRL_REG_DATA_LENGTH (0x1fff << 16) >> +#define LEDC_CTRL_REG_RGB_MODE (0x7 << 6) >> +#define LEDC_CTRL_REG_LEDC_EN BIT(0) >> +#define LEDC_T01_TIMING_CTRL_REG 0x0004 >> +#define LEDC_T01_TIMING_CTRL_REG_T1H (0x3f << 21) >> +#define LEDC_T01_TIMING_CTRL_REG_T1L (0x1f << 16) >> +#define LEDC_T01_TIMING_CTRL_REG_T0H (0x1f << 6) >> +#define LEDC_T01_TIMING_CTRL_REG_T0L (0x3f << 0) >> +#define LEDC_RESET_TIMING_CTRL_REG 0x000c >> +#define LEDC_RESET_TIMING_CTRL_REG_LED_NUM (0x3ff << 0) >> +#define LEDC_DATA_REG 0x0014 >> +#define LEDC_DMA_CTRL_REG 0x0018 >> +#define LEDC_DMA_CTRL_REG_FIFO_TRIG_LEVEL (0x1f << 0) >> +#define LEDC_INT_CTRL_REG 0x001c >> +#define LEDC_INT_CTRL_REG_GLOBAL_INT_EN BIT(5) >> +#define LEDC_INT_CTRL_REG_FIFO_CPUREQ_INT_EN BIT(1) >> +#define LEDC_INT_CTRL_REG_TRANS_FINISH_INT_EN BIT(0) >> +#define LEDC_INT_STS_REG 0x0020 >> +#define LEDC_INT_STS_REG_FIFO_CPUREQ_INT BIT(1) >> +#define LEDC_INT_STS_REG_TRANS_FINISH_INT BIT(0) >> + >> +#define LEDC_FIFO_DEPTH 32 >> +#define LEDC_MAX_LEDS 1024 >> + >> +#define LEDS_TO_BYTES(n) ((n) * sizeof(u32)) >> + >> +struct sunxi_ledc_led { >> + struct led_classdev_mc mc_cdev; >> + struct mc_subled subled_info[3]; >> +}; >> +#define to_ledc_led(mc) container_of(mc, struct sunxi_ledc_led, mc_cdev) >> + >> +struct sunxi_ledc_timing { >> + u32 t0h_ns; >> + u32 t0l_ns; >> + u32 t1h_ns; >> + u32 t1l_ns; >> + u32 treset_ns; >> +}; >> + >> +struct sunxi_ledc { >> + struct device *dev; >> + void __iomem *base; >> + struct clk *bus_clk; >> + struct clk *mod_clk; >> + struct reset_control *reset; >> + struct regulator *vled; >> + >> + u32 *buffer; >> + struct dma_chan *dma_chan; >> + dma_addr_t dma_handle; >> + int pio_length; >> + int pio_offset; >> + >> + spinlock_t lock; >> + int next_length; >> + bool xfer_active; >> + >> + u32 format; >> + struct sunxi_ledc_timing timing; >> + >> + int num_leds; >> + struct sunxi_ledc_led leds[]; >> +}; >> + >> +static int sunxi_ledc_dma_xfer(struct sunxi_ledc *priv, int length) >> +{ >> + struct dma_async_tx_descriptor *desc; >> + dma_cookie_t cookie; >> + >> + desc = dmaengine_prep_slave_single(priv->dma_chan, priv->dma_handle, >> + LEDS_TO_BYTES(length), >> + DMA_MEM_TO_DEV, 0); >> + if (!desc) >> + return -ENOMEM; >> + >> + cookie = dmaengine_submit(desc); >> + if (dma_submit_error(cookie)) >> + return -EIO; >> + >> + dma_async_issue_pending(priv->dma_chan); >> + >> + return 0; >> +} >> + >> +static void sunxi_ledc_pio_xfer(struct sunxi_ledc *priv, int length) >> +{ >> + u32 burst, offset, val; >> + >> + if (length) { >> + /* New transfer (FIFO is empty). */ >> + offset = 0; >> + burst = min(length, LEDC_FIFO_DEPTH); >> + } else { >> + /* Existing transfer (FIFO is half-full). */ >> + length = priv->pio_length; >> + offset = priv->pio_offset; >> + burst = min(length, LEDC_FIFO_DEPTH / 2); >> + } >> + >> + writesl(priv->base + LEDC_DATA_REG, priv->buffer + offset, burst); >> + >> + if (burst < length) { >> + priv->pio_length = length - burst; >> + priv->pio_offset = offset + burst; >> + >> + if (!offset) { >> + val = readl(priv->base + LEDC_INT_CTRL_REG); >> + val |= LEDC_INT_CTRL_REG_FIFO_CPUREQ_INT_EN; >> + writel(val, priv->base + LEDC_INT_CTRL_REG); >> + } >> + } else { >> + /* Disable the request IRQ once all data is written. */ >> + val = readl(priv->base + LEDC_INT_CTRL_REG); >> + val &= ~LEDC_INT_CTRL_REG_FIFO_CPUREQ_INT_EN; >> + writel(val, priv->base + LEDC_INT_CTRL_REG); >> + } >> +} >> + >> +static void sunxi_ledc_start_xfer(struct sunxi_ledc *priv, int length) >> +{ >> + u32 val; >> + >> + dev_dbg(priv->dev, "Updating %d LEDs\n", length); >> + >> + val = readl(priv->base + LEDC_CTRL_REG); >> + val &= ~LEDC_CTRL_REG_DATA_LENGTH; >> + val |= length << 16 | LEDC_CTRL_REG_LEDC_EN; >> + writel(val, priv->base + LEDC_CTRL_REG); >> + >> + if (length > LEDC_FIFO_DEPTH) { >> + int ret = sunxi_ledc_dma_xfer(priv, length); >> + >> + if (!ret) >> + return; >> + >> + dev_warn(priv->dev, "Failed to set up DMA: %d\n", ret); >> + } >> + >> + sunxi_ledc_pio_xfer(priv, length); >> +} >> + >> +static irqreturn_t sunxi_ledc_irq(int irq, void *dev_id) >> +{ >> + struct sunxi_ledc *priv = dev_id; >> + u32 val; >> + >> + val = readl(priv->base + LEDC_INT_STS_REG); >> + >> + if (val & LEDC_INT_STS_REG_TRANS_FINISH_INT) { >> + int next_length; >> + >> + /* Start the next transfer if needed. */ >> + spin_lock(&priv->lock); >> + next_length = priv->next_length; >> + if (next_length) >> + priv->next_length = 0; >> + else >> + priv->xfer_active = false; >> + spin_unlock(&priv->lock); >> + >> + if (next_length) >> + sunxi_ledc_start_xfer(priv, next_length); >> + } else if (val & LEDC_INT_STS_REG_FIFO_CPUREQ_INT) { >> + /* Continue the current transfer. */ >> + sunxi_ledc_pio_xfer(priv, 0); >> + } >> + >> + writel(val, priv->base + LEDC_INT_STS_REG); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static void sunxi_ledc_brightness_set(struct led_classdev *cdev, >> + enum led_brightness brightness) >> +{ >> + struct sunxi_ledc *priv = dev_get_drvdata(cdev->dev->parent); >> + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); >> + struct sunxi_ledc_led *led = to_ledc_led(mc_cdev); >> + int addr = led - priv->leds; >> + unsigned long flags; >> + bool xfer_active; >> + int next_length; >> + >> + led_mc_calc_color_components(mc_cdev, brightness); >> + >> + priv->buffer[addr] = led->subled_info[0].brightness << 16 | >> + led->subled_info[1].brightness << 8 | >> + led->subled_info[2].brightness; >> + >> + dev_dbg(priv->dev, "LED %d -> #%06x\n", addr, priv->buffer[addr]); >> + >> + spin_lock_irqsave(&priv->lock, flags); >> + next_length = max(priv->next_length, addr + 1); >> + xfer_active = priv->xfer_active; >> + if (xfer_active) >> + priv->next_length = next_length; >> + else >> + priv->xfer_active = true; >> + spin_unlock_irqrestore(&priv->lock, flags); >> + >> + if (!xfer_active) >> + sunxi_ledc_start_xfer(priv, next_length); >> +} >> + >> +static const char *const sunxi_ledc_formats[] = { >> + "rgb", >> + "rbg", >> + "grb", >> + "gbr", >> + "brg", >> + "bgr", >> +}; >> + >> +static int sunxi_ledc_parse_format(const struct device_node *np, >> + struct sunxi_ledc *priv) >> +{ >> + const char *format = "grb"; >> + u32 i; >> + >> + of_property_read_string(np, "format", &format); >> + >> + for (i = 0; i < ARRAY_SIZE(sunxi_ledc_formats); ++i) { >> + if (!strcmp(format, sunxi_ledc_formats[i])) { >> + priv->format = i; >> + return 0; >> + } >> + } >> + >> + dev_err(priv->dev, "Bad pixel format '%s'\n", format); >> + >> + return -EINVAL; >> +} >> + >> +static void sunxi_ledc_set_format(struct sunxi_ledc *priv) >> +{ >> + u32 val; >> + >> + val = readl(priv->base + LEDC_CTRL_REG); >> + val &= ~LEDC_CTRL_REG_RGB_MODE; >> + val |= priv->format << 6; >> + writel(val, priv->base + LEDC_CTRL_REG); >> +} >> + >> +static const struct sunxi_ledc_timing sunxi_ledc_default_timing = { >> + .t0h_ns = 336, >> + .t0l_ns = 840, >> + .t1h_ns = 882, >> + .t1l_ns = 294, >> + .treset_ns = 300000, >> +}; >> + >> +static int sunxi_ledc_parse_timing(const struct device_node *np, >> + struct sunxi_ledc *priv) >> +{ >> + struct sunxi_ledc_timing *timing = &priv->timing; >> + >> + *timing = sunxi_ledc_default_timing; >> + >> + of_property_read_u32(np, "t0h-ns", &timing->t0h_ns); >> + of_property_read_u32(np, "t0l-ns", &timing->t0l_ns); >> + of_property_read_u32(np, "t1h-ns", &timing->t1h_ns); >> + of_property_read_u32(np, "t1l-ns", &timing->t1l_ns); >> + of_property_read_u32(np, "treset-ns", &timing->treset_ns); >> + >> + return 0; >> +} >> + >> +static void sunxi_ledc_set_timing(struct sunxi_ledc *priv) >> +{ >> + const struct sunxi_ledc_timing *timing = &priv->timing; >> + unsigned long mod_freq = clk_get_rate(priv->mod_clk); >> + u32 cycle_ns = NSEC_PER_SEC / mod_freq; >> + u32 val; >> + >> + val = (timing->t1h_ns / cycle_ns) << 21 | >> + (timing->t1l_ns / cycle_ns) << 16 | >> + (timing->t0h_ns / cycle_ns) << 6 | >> + (timing->t0l_ns / cycle_ns); >> + writel(val, priv->base + LEDC_T01_TIMING_CTRL_REG); >> + >> + val = (timing->treset_ns / cycle_ns) << 16 | >> + (priv->num_leds - 1); >> + writel(val, priv->base + LEDC_RESET_TIMING_CTRL_REG); >> +} >> + >> +static int sunxi_ledc_resume(struct device *dev) >> +{ >> + struct sunxi_ledc *priv = dev_get_drvdata(dev); >> + u32 val; >> + int ret; >> + >> + ret = regulator_enable(priv->vled); >> + if (ret) >> + return ret; >> + >> + ret = reset_control_deassert(priv->reset); >> + if (ret) >> + goto err_disable_regulator; >> + >> + ret = clk_prepare_enable(priv->bus_clk); >> + if (ret) >> + goto err_assert_reset; >> + >> + ret = clk_prepare_enable(priv->mod_clk); >> + if (ret) >> + goto err_disable_bus_clk; >> + >> + sunxi_ledc_set_format(priv); >> + sunxi_ledc_set_timing(priv); >> + >> + /* The trigger level must be at least the burst length. */ >> + val = readl(priv->base + LEDC_DMA_CTRL_REG); >> + val &= ~LEDC_DMA_CTRL_REG_FIFO_TRIG_LEVEL; >> + val |= LEDC_FIFO_DEPTH / 2; >> + writel(val, priv->base + LEDC_DMA_CTRL_REG); >> + >> + val = LEDC_INT_CTRL_REG_GLOBAL_INT_EN | >> + LEDC_INT_CTRL_REG_TRANS_FINISH_INT_EN; >> + writel(val, priv->base + LEDC_INT_CTRL_REG); >> + >> + return 0; >> + >> +err_disable_bus_clk: >> + clk_disable_unprepare(priv->bus_clk); >> +err_assert_reset: >> + reset_control_assert(priv->reset); >> +err_disable_regulator: >> + regulator_disable(priv->vled); >> + >> + return ret; >> +} >> + >> +static int sunxi_ledc_suspend(struct device *dev) >> +{ >> + struct sunxi_ledc *priv = dev_get_drvdata(dev); >> + >> + clk_disable_unprepare(priv->mod_clk); >> + clk_disable_unprepare(priv->bus_clk); >> + reset_control_assert(priv->reset); >> + regulator_disable(priv->vled); >> + >> + return 0; >> +} >> + >> +static void sunxi_ledc_dma_cleanup(void *data) >> +{ >> + struct sunxi_ledc *priv = data; >> + struct device *dma_dev = dmaengine_get_dma_device(priv->dma_chan); >> + >> + if (priv->buffer) >> + dma_free_wc(dma_dev, LEDS_TO_BYTES(priv->num_leds), >> + priv->buffer, priv->dma_handle); >> + dma_release_channel(priv->dma_chan); >> +} >> + >> +static int sunxi_ledc_probe(struct platform_device *pdev) >> +{ >> + const struct device_node *np = pdev->dev.of_node; >> + struct dma_slave_config dma_cfg = {}; >> + struct led_init_data init_data = {}; >> + struct device *dev = &pdev->dev; >> + struct device_node *child; >> + struct sunxi_ledc *priv; >> + struct resource *mem; >> + int count, irq, ret; >> + >> + count = of_get_available_child_count(np); >> + if (!count) >> + return -ENODEV; >> + if (count > LEDC_MAX_LEDS) { >> + dev_err(dev, "Too many LEDs! (max is %d)\n", LEDC_MAX_LEDS); >> + return -EINVAL; >> + } >> + >> + priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->dev = dev; >> + priv->num_leds = count; >> + spin_lock_init(&priv->lock); >> + dev_set_drvdata(dev, priv); >> + >> + ret = sunxi_ledc_parse_format(np, priv); >> + if (ret) >> + return ret; >> + >> + ret = sunxi_ledc_parse_timing(np, priv); >> + if (ret) >> + return ret; >> + >> + priv->base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); >> + if (IS_ERR(priv->base)) >> + return PTR_ERR(priv->base); >> + >> + priv->bus_clk = devm_clk_get(dev, "bus"); >> + if (IS_ERR(priv->bus_clk)) >> + return PTR_ERR(priv->bus_clk); >> + >> + priv->mod_clk = devm_clk_get(dev, "mod"); >> + if (IS_ERR(priv->mod_clk)) >> + return PTR_ERR(priv->mod_clk); >> + >> + priv->reset = devm_reset_control_get_exclusive(dev, NULL); >> + if (IS_ERR(priv->reset)) >> + return PTR_ERR(priv->reset); >> + >> + priv->vled = devm_regulator_get(dev, "vled"); >> + if (IS_ERR(priv->vled)) >> + return PTR_ERR(priv->vled); >> + >> + priv->dma_chan = dma_request_chan(dev, "tx"); >> + if (IS_ERR(priv->dma_chan)) >> + return PTR_ERR(priv->dma_chan); >> + >> + ret = devm_add_action_or_reset(dev, sunxi_ledc_dma_cleanup, priv); >> + if (ret) >> + return ret; >> + >> + dma_cfg.dst_addr = mem->start + LEDC_DATA_REG; >> + dma_cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; >> + dma_cfg.dst_maxburst = LEDC_FIFO_DEPTH / 2; >> + ret = dmaengine_slave_config(priv->dma_chan, &dma_cfg); >> + if (ret) >> + return ret; >> + >> + priv->buffer = dma_alloc_wc(dmaengine_get_dma_device(priv->dma_chan), >> + LEDS_TO_BYTES(priv->num_leds), >> + &priv->dma_handle, GFP_KERNEL); >> + if (!priv->buffer) >> + return -ENOMEM; >> + >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) >> + return irq; >> + >> + ret = devm_request_irq(dev, irq, sunxi_ledc_irq, 0, dev_name(dev), priv); >> + if (ret) >> + return ret; > > Starting here, you can get interrupts. I couldn't spot anything wrong in > the handler, but would that cause any trouble if an interrupt was coming > right here? It should be fine. The data request IRQ will be disabled if it is received because priv->pio_length == 0, and the other IRQs are oneshots and harmless. >> + ret = sunxi_ledc_resume(dev); >> + if (ret) >> + return ret; > > It's not clear to me what is done here. I assume from the rest of the > driver that you want to use runtime_pm, but it's never enabled for that > driver and the active count will be off here There is no runtime PM here, only system PM. So I have just reused the hardware init procedure from system resume. If I was to add runtime PM, then yes, this call would not belong here. Regards, Samuel