Re: [PATCH v3 2/2] backlight: Add support for Orient Chip OCP8178

[Krzysztof Kozlowski]

On Sun, Aug 16, 2026 at 12:57:04PM +0200, Wim de With wrote:
> Add a driver for the Orient Chip OCP8178 backlight controller.
> 
> The OCP8178 supports either PWM or a vendor-specific onewire protocol
> over GPIO. This driver implements the onewire protocol, which enables
> setting static brightness levels without requiring a continuous PWM
> signal from the host CPU. Note that while the vendor calls the protocol
> 1-Wire, it is not the 1-Wire protocol from Dallas Semiconductor as
> implemented in drivers/w1.
> 
> Signed-off-by: Wim de With <wf@xxxxxxxxx>
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@xxxxxxxxxx>
> ---
>  MAINTAINERS                          |   1 +
>  drivers/video/backlight/Kconfig      |  10 ++
>  drivers/video/backlight/Makefile     |   1 +
>  drivers/video/backlight/ocp8178_bl.c | 231 +++++++++++++++++++++++++++++++++++
>  4 files changed, 243 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3ca99d9c32ef1..7146e4dea8d78 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19682,6 +19682,7 @@ OCP8178 BACKLIGHT DRIVER
>  M:	Wim de With <wf@xxxxxxxxx>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/leds/backlight/ocs,ocp8178.yaml
> +F:	drivers/video/backlight/ocp8178_bl.c
>  
>  OCXL (Open Coherent Accelerator Processor Interface OpenCAPI) DRIVER
>  M:	Mahesh J Salgaonkar <mahesh@xxxxxxxxxxxxx>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 7aa1c4b21111f..aa845230c5f58 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -226,6 +226,16 @@ config BACKLIGHT_LOCOMO
>  	  If you have a Sharp Zaurus SL-5500 (Collie) or SL-5600 (Poodle) say y to
>  	  enable the LCD/backlight driver.
>  
> +config BACKLIGHT_OCP8178
> +	tristate "OCP8178 Backlight Driver"
> +	depends on GPIOLIB
> +	help
> +	  If you have an Orient Chip OCP8178, say Y to enable the backlight
> +	  driver.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called ocp8178_bl.
> +
>  config BACKLIGHT_OMAP1
>  	tristate "OMAP1 PWL-based LCD Backlight"
>  	depends on ARCH_OMAP1 || COMPILE_TEST
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 21c8313cfb121..c4e87b708ae21 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_BACKLIGHT_MAX25014)	+= max25014.o
>  obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
>  obj-$(CONFIG_BACKLIGHT_MP3309C)		+= mp3309c.o
>  obj-$(CONFIG_BACKLIGHT_MT6370)		+= mt6370-backlight.o
> +obj-$(CONFIG_BACKLIGHT_OCP8178)		+= ocp8178_bl.o
>  obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
>  obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
>  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
> diff --git a/drivers/video/backlight/ocp8178_bl.c b/drivers/video/backlight/ocp8178_bl.c
> new file mode 100644
> index 0000000000000..6c1e8c9451f12
> --- /dev/null
> +++ b/drivers/video/backlight/ocp8178_bl.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Orient Chip OCP8178 Backlight Driver
> + *
> + * Copyright (C) 2026 Wim de With
> + *
> + * Author: Wim de With <wf@xxxxxxxxx>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/irqflags.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +
> +#define OCP8178_MAX_BRIGHTNESS 0x1F /* 5 bits */
> +
> +#define OCP8178_DEVICE_ADDRESS 0x72
> +
> +#define OCP8178_DATA_RFA BIT(7)
> +#define OCP8178_DATA_ADDR GENMASK(6, 5)
> +#define OCP8178_DATA_VALUE GENMASK(4, 0)
> +
> +#define OCP8178_1W_INIT_MAX_RETRIES 5
> +#define OCP8178_1W_INIT_SLEEP_US (50 * 1000)
> +
> +#define OCP8178_T_OFF_US (3 * 1000) /* datasheet specifies at least 2.5 ms */
> +#define OCP8178_1W_T_DELAY_US (100 + 10) /* 10 us as safety factor */
> +#define OCP8178_1W_T_DETECT_US (260 + 10) /* 10 us as safety factor */
> +#define OCP8178_1W_T_START_US 2
> +#define OCP8178_1W_T_EOS_US 2
> +#define OCP8178_1W_T_WIN_NS (1000 * 1000)
> +
> +/*
> + * The datasheet specifies 1.7 Kbps to 160 Kbps.
> + * 1 / (160 Kbps) is about 6.67 us, so using 7 us per bit should be fine.
> + * T_HIGH + T_LOW = 7 us
> + * T_HIGH > 2 * T_LOW for high bits
> + * T_LOW > 2 * T_HIGH for low bits
> + */
> +#define OCP8178_1W_HIGH_BIT_T_LOW_US 2
> +#define OCP8178_1W_HIGH_BIT_T_HIGH_US 5
> +#define OCP8178_1W_LOW_BIT_T_LOW_US 5
> +#define OCP8178_1W_LOW_BIT_T_HIGH_US 2
> +
> +struct ocp8178_bl {
> +	struct device *dev;
> +	struct gpio_desc *gpiod;
> +};
> +
> +static int ocp8178_bl_enable_onewire(struct ocp8178_bl *ocp8178)
> +{
> +	u64 start, duration;
> +

It seems you re-implemented one-wire interface instead of using w1. Why?
What sort of one-wire bus/interface is this?

Best regards,
Krzysztof