Re: [Outreachy kernel] [PATCH v13 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 24, 2017 at 01:51:45PM +0100, Daniel Thompson wrote:
> On 24/10/17 13:16, Sean Paul wrote:
> > On Mon, Oct 23, 2017 at 11:35:42AM +0100, Daniel Thompson wrote:
> > > On 20/10/17 18:20, Sean Paul wrote:
> > > > On Wed, Oct 18, 2017 at 1:11 PM, Meghana Madhyastha
> > > > <meghana.madhyastha@xxxxxxxxx> wrote:
> > > > > On Mon, Oct 16, 2017 at 02:26:00PM -0400, Sean Paul wrote:
> > > > > > On Fri, Oct 13, 2017 at 6:42 PM, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:
> > > > > > > 
> > > > > > > Den 13.10.2017 22.25, skrev Sean Paul:
> > > > > > > > 
> > > > > > > > On Fri, Oct 13, 2017 at 04:11:43PM +0530, Meghana Madhyastha wrote:
> > > > > > > > > 
> > > > > > > > > Rename tinydrm_of_find_backlight to backlight_get and move it
> > > > > > > > > to linux/backlight.c so that it can be used by other drivers.
> > > > > > > > 
> > > > > > > > [apologies if this has been brought up in previous versions, I haven't
> > > > > > > > been
> > > > > > > > following closely]
> > > > > > > > 
> > > > > > > > I don't think "backlight_get" is a good name for this function. How about
> > > > > > > > of_find_backlight_by_name (since there's already
> > > > > > > > of_find_backlight_by_node)?
> > > > > > > 
> > > > > > > 
> > > > > > > I came up with that name modelled after gpiod_get() and gpiod_put() and I
> > > > > > > deliberately kept the of_ part out of the name like the gpio functions.
> > > > > > > gpiod_get() checks OF, ACPI and platform for gpios and calling it
> > > > > > > backlight_get() would keep the door open for other ways of connecting
> > > > > > > backlight devices in the future, other than Device Tree.
> > > > > > > 
> > > > > > 
> > > > > > Thanks for the background, Noralf! Apologies for stepping on top of
> > > > > > your previous reviews.
> > > > > > 
> > > > > > > I think of_find_backlight() would be better than *_by_name(), since
> > > > > > > 'backlight' is the common DT property name, so it wouldn't make much sense
> > > > > > > to require every caller to pass in the same name.
> > > > > > > 
> > > > > > 
> > > > > > of_find_backlight() sounds like a good compromise.
> > > > > > 
> > > > > > Sean
> > > > > 
> > > > > Are there any changes that need to be made to this patchset now ?
> > > > > 
> > > > 
> > > > I still prefer s/backlight_get/of_find_backlight/ and it seems Noralf
> > > > is Ok with that too. I think there were also review comments
> > > > surrounding the _put function?
> > > 
> > > How did this conversation result in a new patchset with the name changed?
> > > 
> > > Did I miss something.
> > > 
> > 
> > I asked the name be changed to of_find_backlight and that backlight_put be
> > removed.
> 
> Quite so.
> 
> Sorry, I was filtering my mail way too fast and misread your comment as "I
> still prefer backlight_get" :-O

I can certainly relate to that problem! I'm happy we got this sorted out :-)

Sean

> That was my problem with reading, not your problem with writing.
> 
> 
> Daniel.

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux