Re: [PATCH v2 13/15] backlight: rave-sp: Include <linux/of.h> and <linux/mod_devicetable.h>

[Daniel Thompson <danielt@kernel.org>]

On Wed, Jul 16, 2025 at 08:39:03AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 15.07.25 um 15:34 schrieb Rob Herring:
> > On Tue, Jul 15, 2025 at 7:30 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > > Include <linux/of.h> to declare struct device_node and include
> > > <linux/mod_devicetable.h> to declare struct of_device_id. Avoids
> > > dependency on backlight header to include it.
> > struct device_node should be opaque...
> >
> >          /*
> >           * If there is a phandle pointing to the device node we can
> >           * assume that another device will manage the status changes.
> >           * If not we make sure the backlight is in a consistent state.
> >           */
> >          if (!dev->of_node->phandle)
> >                  backlight_update_status(bd);
> >
> > Well, that is ugly. IMO, we should just drop the check. A DT built
> > with "-@" option will have phandle set, so that's not a reliable test.
>
> Not that I disagree, but fixing it is out of scope for this series.

This pattern appears in several places. It is used to guess whether
the initial state of the backlight should be on or off (which avoids
flickering artefacts during boot).

You won't get much argument from me about the ugliness either. However
this patch makes the presence of this logic more obvious rather than
less. On that basis I view it as moving in the right direction rather
than the wrong one and therefore:
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>


Daniel.