Re: [PATCH 00/10] fujitsu-laptop: use device-specific data instead of module-wide globals

From: MichaÅ KÄpieÅ
Date: Tue May 16 2017 - 02:41:01 EST


> On Mon, May 15, 2017 at 04:27:25PM -0700, Darren Hart wrote:
> > > In light of the above, I still feel the split is worth going through
> > > with. The question is whether Jonathan feels the same :)
> >
> > In the interest of keeping this moving... As I'm not sure there is a "right
> > answer" to split or not, and nobody screamed out against splitting, and this is
> > the direction Michal seems to prefer, and he is doing the work, let's proceed
> > with the split of -backlight and -laptop.
>
> Apologies for not getting back about this earlier. As mentioned in my
> follow up to Michael's post from a few minutes ago I agree with the above
> sentiment.
>
> > > Jonathan, assuming the objective of splitting the module in two, allow
> > > me to pick your brain a bit:
> > >
> > > 1. Would you be okay with leaving "priv" as the variable name for
> > > device-specific data in both drivers? If they are to be separated,
> > > "priv" would soon become unambiguous. I do not have any strong
> > > feelings about this, though.
> > >
> > > 2. Would you be okay with renaming "acpi_handle" to "handle"? Darren
> > > seems to like this idea and in light of the above we would not have
> > > another ACPI handle inside struct fujitsu_bl any more.
> >
> > Both of these are easily discussed in the next series which will most likely
> > have at least one respin anyway.
>
> Assuming the split happens I am happy with both of these proposals. The
> concerns raised earlier were precipitated mostly because I was unaware of
> the medium term goal of splitting the driver (not because it hadn't been
> mentioned, but because I had forgotten about it in the time since it was
> first raised earlier in the year).
>
> > > 3. You mentioned earlier that you were not really fond of the fext_*()
> > > helper functions. Would you like me to drop them and simply use
> > > call_fext_func() with five arguments everywhere? Or should I keep
> > > the helper functions in v2?
> >
> > I was torn on this as well - I didn't think they added much value. Let's
> > focus on splitting the driver, and we can revisit this later for the
> > -laptop driver if there is interest.
>
> It seems I misinterpreted Darren's stance on this one and misrepresented him
> in my previous post (sorry Darren). Since Darren's preferred approach
> is to drop them for the moment let's run with that. As he said, once the
> split has been made we can obviously revisit this to see if there value in
> using them in the context of the split drivers.

Jonathan, Darren, thank you for all the feedback. Silence on my behalf
has not been coincidental as I have also been busy lately and had to put
kernel stuff on the back burner. Sadly, I can also now confirm that I
will no longer have access to the E744 I used to test my patches on as
of next Monday. I will do my best to prepare v2 of this series before
that.

--
Best regards,
MichaÅ KÄpieÅ