Re: [PATCH v3 3/3] backlight: Add Kinetic KTD2801 backlight support

From: Duje Mihanović
Date: Mon Jan 22 2024 - 12:24:24 EST


On Monday, January 22, 2024 11:28:05 AM CET Daniel Thompson wrote:
> On Sat, Jan 20, 2024 at 10:26:45PM +0100, Duje Mihanović wrote:
> > diff --git a/drivers/video/backlight/ktd2801-backlight.c
> > b/drivers/video/backlight/ktd2801-backlight.c new file mode 100644
> > index 000000000000..7b9d1a93aa71
> > --- /dev/null
> > <snip>
> > +/* These values have been extracted from Samsung's driver. */
> > +#define KTD2801_EXPRESSWIRE_DETECT_DELAY_US 150
> > +#define KTD2801_EXPRESSWIRE_DETECT_US 270
> > +#define KTD2801_SHORT_BITSET_US 5
> > +#define KTD2801_LONG_BITSET_US (3 *
KTD2801_SHORT_BITSET_US)
> > +#define KTD2801_DATA_START_US 5
> > +#define KTD2801_END_OF_DATA_LOW_US 10
> > +#define KTD2801_END_OF_DATA_HIGH_US 350
> > +#define KTD2801_PWR_DOWN_DELAY_US 2600
>
> These are a little pointless now. They are all single use constants
> and have little documentary value.
>
> The lack of documentary value is because, for example,
> KTD2801_EXPRESSWIRE_DETECT_DELAY_US, is assigned to a structure
> field called detect_delay_us.
>
> Likewise I doubt that explicitly stating that long_bitset_us is 3x
> bigger than short_bitset_us is important for future driver maintainance.

Does this apply for ktd2692 as well?

Regards,
--
Duje