Hello Lee, Am Freitag, dem 21.06.2024 um 11:26 +0100 schrieb Lee Jones: > On Sun, 16 Jun 2024, André Apitzsch via B4 Relay wrote: > > > From: André Apitzsch <git@xxxxxxxxxxx> > > > > The SY7802 is a current-regulated charge pump which can regulate > > two > > current levels for Flash and Torch modes. > > > > It is a high-current synchronous boost converter with 2-channel > > high > > side current sources. Each channel is able to deliver 900mA > > current. > > > > Signed-off-by: André Apitzsch <git@xxxxxxxxxxx> > > --- > > drivers/leds/flash/Kconfig | 11 + > > drivers/leds/flash/Makefile | 1 + > > drivers/leds/flash/leds-sy7802.c | 542 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 554 insertions(+) > > Generally very nice. > > Just a couple of teensy nits to fix then add my and resubmit please. > > Acked-by: Lee Jones <lee@xxxxxxxxxx> > > > [...] > > diff --git a/drivers/leds/flash/leds-sy7802.c > > b/drivers/leds/flash/leds-sy7802.c > > new file mode 100644 > > index 000000000000..c4bea55a62d0 > > --- /dev/null > > +++ b/drivers/leds/flash/leds-sy7802.c > > @@ -0,0 +1,542 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Silergy SY7802 flash LED driver with I2C interface > > "an I2C interface" > > Or > > "I2C interfaces" > > > + * Copyright 2024 André Apitzsch <git@xxxxxxxxxxx> > > + */ > > + > > +#include <linux/gpio/consumer.h> > > +#include <linux/i2c.h> > > +#include <linux/kernel.h> > > +#include <linux/led-class-flash.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > + > > +#define SY7802_MAX_LEDS 2 > > +#define SY7802_LED_JOINT 2 > > + > > +#define SY7802_REG_ENABLE 0x10 > > +#define SY7802_REG_TORCH_BRIGHTNESS 0xa0 > > +#define SY7802_REG_FLASH_BRIGHTNESS 0xb0 > > +#define SY7802_REG_FLASH_DURATION 0xc0 > > +#define SY7802_REG_FLAGS 0xd0 > > +#define SY7802_REG_CONFIG_1 0xe0 > > +#define SY7802_REG_CONFIG_2 0xf0 > > +#define SY7802_REG_VIN_MONITOR 0x80 > > +#define SY7802_REG_LAST_FLASH 0x81 > > +#define SY7802_REG_VLED_MONITOR 0x30 > > +#define SY7802_REG_ADC_DELAY 0x31 > > +#define SY7802_REG_DEV_ID 0xff > > + > > +#define SY7802_MODE_OFF 0 > > +#define SY7802_MODE_TORCH 2 > > +#define SY7802_MODE_FLASH 3 > > +#define SY7802_MODE_MASK GENMASK(1, 0) > > + > > +#define SY7802_LEDS_SHIFT 3 > > +#define SY7802_LEDS_MASK(_id) (BIT(_id) << SY7802_LEDS_SHIFT) > > +#define SY7802_LEDS_MASK_ALL (SY7802_LEDS_MASK(0) | > > SY7802_LEDS_MASK(1)) > > + > > +#define SY7802_TORCH_CURRENT_SHIFT 3 > > +#define SY7802_TORCH_CURRENT_MASK(_id) \ > > + (GENMASK(2, 0) << (SY7802_TORCH_CURRENT_SHIFT * (_id))) > > +#define SY7802_TORCH_CURRENT_MASK_ALL \ > > + (SY7802_TORCH_CURRENT_MASK(0) | > > SY7802_TORCH_CURRENT_MASK(1)) > > + > > +#define SY7802_FLASH_CURRENT_SHIFT 4 > > +#define SY7802_FLASH_CURRENT_MASK(_id) \ > > + (GENMASK(3, 0) << (SY7802_FLASH_CURRENT_SHIFT * (_id))) > > +#define SY7802_FLASH_CURRENT_MASK_ALL \ > > + (SY7802_FLASH_CURRENT_MASK(0) | > > SY7802_FLASH_CURRENT_MASK(1)) > > + > > +#define SY7802_TIMEOUT_DEFAULT_US 512000U > > +#define SY7802_TIMEOUT_MIN_US 32000U > > +#define SY7802_TIMEOUT_MAX_US 1024000U > > +#define SY7802_TIMEOUT_STEPSIZE_US 32000U > > + > > +#define SY7802_TORCH_BRIGHTNESS_MAX 8 > > + > > +#define SY7802_FLASH_BRIGHTNESS_DEFAULT 14 > > +#define SY7802_FLASH_BRIGHTNESS_MIN 0 > > +#define SY7802_FLASH_BRIGHTNESS_MAX 15 > > +#define SY7802_FLASH_BRIGHTNESS_STEP 1 > > + > > +#define SY7802_FLAG_TIMEOUT BIT(0) > > +#define SY7802_FLAG_THERMAL_SHUTDOWN BIT(1) > > +#define SY7802_FLAG_LED_FAULT BIT(2) > > +#define SY7802_FLAG_TX1_INTERRUPT BIT(3) > > +#define SY7802_FLAG_TX2_INTERRUPT BIT(4) > > +#define SY7802_FLAG_LED_THERMAL_FAULT BIT(5) > > +#define SY7802_FLAG_FLASH_INPUT_VOLTAGE_LOW BIT(6) > > +#define SY7802_FLAG_INPUT_VOLTAGE_LOW BIT(7) > > + > > +#define SY7802_CHIP_ID 0x51 > > + > > +static const struct reg_default sy7802_regmap_defs[] = { > > + { SY7802_REG_ENABLE, SY7802_LEDS_MASK_ALL }, > > + { SY7802_REG_TORCH_BRIGHTNESS, 0x92 }, > > + { SY7802_REG_FLASH_BRIGHTNESS, > > SY7802_FLASH_BRIGHTNESS_DEFAULT | > > + SY7802_FLASH_BRIGHTNESS_DEFAULT << > > SY7802_FLASH_CURRENT_SHIFT }, > > + { SY7802_REG_FLASH_DURATION, 0x6f }, > > + { SY7802_REG_FLAGS, 0x0 }, > > + { SY7802_REG_CONFIG_1, 0x68 }, > > + { SY7802_REG_CONFIG_2, 0xf0 }, > > Not your fault, but this interface is frustrating since we have no > idea > what these register values mean. IMHO, they should be defined and > ORed > together in some human readable way. > > I say that it's not your fault because I see that this is the most > common usage. > I don't know how to interpret some bits of the default values. I don't have the documentation and changing the bits and observing the behavior of the device also didn't help. Should I remove the entries from sy7802_regmap_defs, which have values that we don't fully understand? Regards, André