[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bqn4tddl3kgle7zlamgaqlh45pizw6gf5qjwlmcsbkb6fx343l@tf5w63xursi2>
Date: Mon, 9 Dec 2024 10:22:17 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nick Chan <towinchenmi@...il.com>,
y@...k-bin.smtp.subspace.kernel.org
Cc: Lee Jones <lee@...nel.org>, Daniel Thompson <danielt@...nel.org>,
Jingoo Han <jingoohan1@...il.com>, Pavel Machek <pavel@....cz>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Helge Deller <deller@....de>,
Hector Martin <marcan@...can.st>, Sven Peter <sven@...npeter.dev>,
Alyssa Rosenzweig <alyssa@...enzweig.io>, dri-devel@...ts.freedesktop.org, linux-leds@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/3] backlight: dwi_bl: Add Apple DWI backlight driver
On Sat, Dec 07, 2024 at 09:03:15PM +0800, Nick Chan wrote:
> Add driver for backlight controllers attached via Apple DWI 2-wire
> interface, which is found on some Apple iPhones, iPads and iPod touches
> with a LCD display.
>
> Signed-off-by: Nick Chan <towinchenmi@...il.com>
> ---
> drivers/video/backlight/Kconfig | 12 +++
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/dwi_bl.c | 124 +++++++++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 drivers/video/backlight/dwi_bl.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 3614a5d29c71..e64cc3d51ac5 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -166,6 +166,18 @@ config BACKLIGHT_EP93XX
> To compile this driver as a module, choose M here: the module will
> be called ep93xx_bl.
>
> +config BACKLIGHT_APPLE_DWI
> + tristate "Apple DWI 2-Wire Interface Backlight Driver"
> + depends on ARCH_APPLE || COMPILE_TEST
> + default y
> + help
> + Say Y to enable the backlight driver for backlight controllers
> + attached via the Apple DWI 2-wire interface which is found in some
> + Apple iPhones, iPads and iPod touches.
Mixed/messed indentation. Some spaces?
> +
> + To compile this driver as a module, choose M here: the module will
> + be called dwi_bl.
> +
> config BACKLIGHT_IPAQ_MICRO
> tristate "iPAQ microcontroller backlight driver"
> depends on MFD_IPAQ_MICRO
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 8fc98f760a8a..0a569d7f0210 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o
> obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
> obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
> obj-$(CONFIG_BACKLIGHT_DA9052) += da9052_bl.o
> +obj-$(CONFIG_BACKLIGHT_APPLE_DWI) += dwi_bl.o
Please do not introduce other sorting way - it is sorted by config name.
Which will also point you to apple_bl.o and need of explaining the
differences, e.g. why this cannot be one driver.
> obj-$(CONFIG_BACKLIGHT_EP93XX) += ep93xx_bl.o
> obj-$(CONFIG_BACKLIGHT_GPIO) += gpio_backlight.o
> obj-$(CONFIG_BACKLIGHT_HP680) += hp680_bl.o
> diff --git a/drivers/video/backlight/dwi_bl.c b/drivers/video/backlight/dwi_bl.c
> new file mode 100644
> index 000000000000..d4bfd74b3129
> --- /dev/null
> +++ b/drivers/video/backlight/dwi_bl.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Driver for backlight controllers attached via Apple DWI 2-wire interface
> + *
> + * Copyright (c) 2024 Nick Chan <towinchenmi@...il.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/backlight.h>
> +
> +#define DWI_BL_CTL 0x0
> +#define DWI_BL_CTL_SEND1 BIT(0)
> +#define DWI_BL_CTL_SEND2 BIT(4)
> +#define DWI_BL_CTL_SEND3 BIT(5)
> +#define DWI_BL_CTL_LE_DATA BIT(6)
> +/* Only used on Apple A9 and later */
> +#define DWI_BL_CTL_SEND4 BIT(12)
> +
> +#define DWI_BL_CMD 0x4
> +#define DWI_BL_CMD_TYPE GENMASK(31, 28)
> +#define DWI_BL_CMD_TYPE_SET_BRIGHTNESS 0xa
> +#define DWI_BL_CMD_DATA GENMASK(10, 0)
> +
> +#define DWI_BL_CTL_SEND (DWI_BL_CTL_SEND1 | \
> + DWI_BL_CTL_SEND2 | \
> + DWI_BL_CTL_SEND3 | \
> + DWI_BL_CTL_LE_DATA | \
> + DWI_BL_CTL_SEND4)
> +
> +#define DWI_BL_MAX_BRIGHTNESS 2047
> +
> +struct apple_dwi_bl {
> + void __iomem *base;
> +};
> +
> +static int dwi_bl_update_status(struct backlight_device *bl)
> +{
> + struct apple_dwi_bl *dwi_bl = bl_get_data(bl);
> +
> + int brightness = backlight_get_brightness(bl);
> +
> + u32 cmd = 0;
> +
> + cmd |= FIELD_PREP(DWI_BL_CMD_DATA, brightness);
> + cmd |= FIELD_PREP(DWI_BL_CMD_TYPE, DWI_BL_CMD_TYPE_SET_BRIGHTNESS);
> +
> + writel(cmd, dwi_bl->base + DWI_BL_CMD);
> + writel(DWI_BL_CTL_SEND, dwi_bl->base + DWI_BL_CTL);
> +
> + return 0;
> +}
> +
> +static int dwi_bl_get_brightness(struct backlight_device *bl)
> +{
> + struct apple_dwi_bl *dwi_bl = bl_get_data(bl);
> +
> + u32 cmd = readl(dwi_bl->base + DWI_BL_CMD);
> +
> + return FIELD_GET(DWI_BL_CMD_DATA, cmd);
> +}
> +
> +static const struct backlight_ops dwi_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .get_brightness = dwi_bl_get_brightness,
> + .update_status = dwi_bl_update_status
> +};
> +
> +static int dwi_bl_probe(struct platform_device *dev)
> +{
> + struct apple_dwi_bl *dwi_bl;
> + struct backlight_device *bl;
> + struct backlight_properties props;
> + struct resource *res;
> +
> + dwi_bl = devm_kzalloc(&dev->dev, sizeof(struct apple_dwi_bl), GFP_KERNEL);
sizeof(*)
> + if (!dwi_bl)
> + return -ENOMEM;
> +
> + res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENXIO;
> +
> + dwi_bl->base = devm_ioremap_resource(&dev->dev, res);
Use helper for these two, devm_platform_get_and_ioremap_resource() or
similar.
Best regards,
Krzysztof
Powered by blists - more mailing lists