From: "Duje Mihanović" <duje.mihanovic@skole.hr>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Lee Jones <lee@kernel.org>, Jingoo Han <jingoohan1@gmail.com>,
Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>, Helge Deller <deller@gmx.de>,
Linus Walleij <linus.walleij@linaro.org>,
Karel Balej <balejk@matfyz.cz>,
~postmarketos/upstreaming@lists.sr.ht,
phone-devel@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v3 3/3] backlight: Add Kinetic KTD2801 backlight support
Date: Mon, 22 Jan 2024 17:24:56 +0100 [thread overview]
Message-ID: <1783156.VLH7GnMWUR@radijator> (raw)
In-Reply-To: <20240122102805.GB8596@aspen.lan>
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
WARNING: multiple messages have this Message-ID (diff)
From: "Duje Mihanović" <duje.mihanovic@skole.hr>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: devicetree@vger.kernel.org, Conor Dooley <conor+dt@kernel.org>,
Pavel Machek <pavel@ucw.cz>, Jingoo Han <jingoohan1@gmail.com>,
Helge Deller <deller@gmx.de>, Lee Jones <lee@kernel.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Karel Balej <balejk@matfyz.cz>,
linux-fbdev@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
~postmarketos/upstreaming@lists.sr.ht,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
phone-devel@vger.kernel.org, linux-leds@vger.kernel.org
Subject: Re: [PATCH v3 3/3] backlight: Add Kinetic KTD2801 backlight support
Date: Mon, 22 Jan 2024 17:24:56 +0100 [thread overview]
Message-ID: <1783156.VLH7GnMWUR@radijator> (raw)
In-Reply-To: <20240122102805.GB8596@aspen.lan>
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
next prev parent reply other threads:[~2024-01-22 16:25 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-20 21:26 [PATCH v3 0/3] Kinetic ExpressWire library and KTD2801 backlight driver Duje Mihanović
2024-01-20 21:26 ` Duje Mihanović
2024-01-20 21:26 ` [PATCH v3 1/3] leds: ktd2692: move ExpressWire code to library Duje Mihanović
2024-01-20 21:26 ` Duje Mihanović
2024-01-21 14:35 ` Linus Walleij
2024-01-21 14:35 ` Linus Walleij
2024-01-21 15:06 ` Duje Mihanović
2024-01-21 15:06 ` Duje Mihanović
2024-01-22 10:19 ` Daniel Thompson
2024-01-22 10:19 ` Daniel Thompson
2024-01-22 16:24 ` Duje Mihanović
2024-01-22 16:24 ` Duje Mihanović
2024-01-22 16:50 ` Daniel Thompson
2024-01-22 16:50 ` Daniel Thompson
2024-01-22 16:57 ` Duje Mihanović
2024-01-22 16:57 ` Duje Mihanović
2024-01-22 17:26 ` Duje Mihanović
2024-01-22 17:26 ` Duje Mihanović
2024-01-22 17:50 ` Daniel Thompson
2024-01-22 17:50 ` Daniel Thompson
2024-01-22 18:13 ` Duje Mihanović
2024-01-22 18:13 ` Duje Mihanović
2024-01-20 21:26 ` [PATCH v3 2/3] dt-bindings: backlight: add Kinetic KTD2801 binding Duje Mihanović
2024-01-20 21:26 ` Duje Mihanović
2024-01-20 21:26 ` [PATCH v3 3/3] backlight: Add Kinetic KTD2801 backlight support Duje Mihanović
2024-01-20 21:26 ` Duje Mihanović
2024-01-21 14:37 ` Linus Walleij
2024-01-21 14:37 ` Linus Walleij
2024-01-22 10:28 ` Daniel Thompson
2024-01-22 10:28 ` Daniel Thompson
2024-01-22 16:24 ` Duje Mihanović [this message]
2024-01-22 16:24 ` Duje Mihanović
2024-01-22 16:51 ` Daniel Thompson
2024-01-22 16:51 ` Daniel Thompson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1783156.VLH7GnMWUR@radijator \
--to=duje.mihanovic@skole.hr \
--cc=balejk@matfyz.cz \
--cc=conor+dt@kernel.org \
--cc=daniel.thompson@linaro.org \
--cc=deller@gmx.de \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jingoohan1@gmail.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lee@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=phone-devel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=~postmarketos/upstreaming@lists.sr.ht \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.