Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1250220 > unrolled thread

Re: [GIT PULL] On-demand device probing

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-18 21:40 +0200
    Re: [GIT PULL] On-demand device probing Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-18 21:40 +0200
      Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-18 22:00 +0200
        Re: [GIT PULL] On-demand device probing David Woodhouse <dwmw2@infradead.org> - 2015-10-19 11:50 +0200
          Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 12:00 +0200
          Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-19 13:10 +0200
          Re: [GIT PULL] On-demand device probing Rob Herring <robh+dt@kernel.org> - 2015-10-19 14:40 +0200
            Re: [GIT PULL] On-demand device probing David Woodhouse <dwmw2@infradead.org> - 2015-10-19 14:50 +0200
              Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-19 17:00 +0200
                Re: [GIT PULL] On-demand device probing David Woodhouse <dwmw2@infradead.org> - 2015-10-19 17:40 +0200
                  Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 17:50 +0200
                    Re: [GIT PULL] On-demand device probing Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-10-19 20:30 +0200
                      Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 20:50 +0200
                        Re: [GIT PULL] On-demand device probing Alexandre Courbot <gnurou@gmail.com> - 2015-10-20 01:50 +0200
                          Re: gpiod API considerations [Was: [GIT PULL] On-demand device  probing] Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-10-20 09:20 +0200
                    Re: [GIT PULL] On-demand device probing David Woodhouse <dwmw2@infradead.org> - 2015-10-20 13:20 +0200
                  Re: [GIT PULL] On-demand device probing Rob Herring <robh+dt@kernel.org> - 2015-10-19 18:00 +0200
                    Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-19 23:20 +0200
                      Re: [GIT PULL] On-demand device probing Rob Herring <robh@kernel.org> - 2015-10-20 01:00 +0200
                        Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-20 09:30 +0200
                          Re: [GIT PULL] On-demand device probing Rob Herring <robh@kernel.org> - 2015-10-20 16:20 +0200
                            Re: [GIT PULL] On-demand device probing Alan Stern <stern@rowland.harvard.edu> - 2015-10-20 16:50 +0200
                              Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-20 17:40 +0200
                                Re: [GIT PULL] On-demand device probing Alan Stern <stern@rowland.harvard.edu> - 2015-10-20 18:10 +0200
                                  Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-20 18:30 +0200
                                    Re: [GIT PULL] On-demand device probing Alan Stern <stern@rowland.harvard.edu> - 2015-10-20 19:20 +0200
                                      Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-20 21:40 +0200
                                        Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-21 01:10 +0200
                                          Re: [GIT PULL] On-demand device probing Jean-Francois Moine <moinejf@free.fr> - 2015-10-21 08:20 +0200
                                    Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-22 02:30 +0200
                                      Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-22 11:20 +0200
                                  Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-27 05:40 +0100
                            Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-21 01:10 +0200
                              Re: [GIT PULL] On-demand device probing Geert Uytterhoeven <geert@linux-m68k.org> - 2015-10-21 11:00 +0200
                                Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-22 01:20 +0200
                  Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-19 18:10 +0200
        Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-19 14:40 +0200
          Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 15:20 +0200
            Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-19 16:20 +0200
              Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 16:40 +0200
                Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-19 17:10 +0200
                  Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 17:40 +0200
                    Re: [GIT PULL] On-demand device probing Geert Uytterhoeven <geert@linux-m68k.org> - 2015-10-19 18:30 +0200
                      Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 18:50 +0200
                      Re: Alternative approach to solve the deferred probe (was: [GIT  PULL] On-demand device probing) Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-20 17:50 +0200
                        Re: Alternative approach to solve the deferred probe Frank Rowand <frowand.list@gmail.com> - 2015-10-21 06:00 +0200
                          Re: Alternative approach to solve the deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-21 10:20 +0200
                            Re: Alternative approach to solve the deferred probe Frank Rowand <frowand.list@gmail.com> - 2015-10-21 17:40 +0200
                              Re: Alternative approach to solve the deferred probe Grygorii Strashko <grygorii.strashko@ti.com> - 2015-10-21 19:00 +0200
                                Re: Alternative approach to solve the deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-21 19:30 +0200
                                  Re: Alternative approach to solve the deferred probe Grygorii Strashko <grygorii.strashko@ti.com> - 2015-10-21 20:20 +0200
                                    Re: Alternative approach to solve the deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-21 20:30 +0200
                                      Re: Alternative approach to solve the deferred probe Grygorii Strashko <grygorii.strashko@ti.com> - 2015-10-22 17:20 +0200
                                Re: Alternative approach to solve the deferred probe Frank Rowand <frowand.list@gmail.com> - 2015-10-21 20:10 +0200
                                  Re: Alternative approach to solve the deferred probe Grygorii Strashko <grygorii.strashko@ti.com> - 2015-10-21 20:40 +0200
                              Re: Alternative approach to solve the deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-21 22:40 +0200
                                Re: Alternative approach to solve the deferred probe Frank Rowand <frowand.list@gmail.com> - 2015-10-22 02:10 +0200
                        Re: Alternative approach to solve the deferred probe (was: [GIT  PULL] On-demand device probing) Mark Brown <broonie@kernel.org> - 2015-10-23 01:40 +0200
          Re: [GIT PULL] On-demand device probing Frank Rowand <frowand.list@gmail.com> - 2015-10-21 18:10 +0200
            Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-21 18:30 +0200
              Re: [GIT PULL] On-demand device probing Frank Rowand <frowand.list@gmail.com> - 2015-10-21 20:30 +0200
                Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-21 23:10 +0200
                Re: [GIT PULL] On-demand device probing Rob Herring <robh+dt@kernel.org> - 2015-10-21 23:20 +0200
                  Re: [GIT PULL] On-demand device probing Frank Rowand <frowand.list@gmail.com> - 2015-10-22 00:00 +0200
                    Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-22 11:10 +0200
                      Re: [GIT PULL] On-demand device probing Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-22 16:40 +0200
                      Re: [GIT PULL] On-demand device probing Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-22 16:50 +0200
                        Re: [GIT PULL] On-demand device probing Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-22 17:10 +0200
                          Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-23 01:40 +0200
                        Re: [GIT PULL] On-demand device probing Frank Rowand <frowand.list@gmail.com> - 2015-10-22 21:00 +0200
                          Re: [GIT PULL] On-demand device probing Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-22 21:30 +0200
                          Re: [GIT PULL] On-demand device probing Tim Bird <tim.bird@sonymobile.com> - 2015-10-23 17:50 +0200
                            Re: [GIT PULL] On-demand device probing Rob Herring <robh+dt@kernel.org> - 2015-10-23 18:40 +0200
                              Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-24 15:50 +0200
                                Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-25 00:10 +0200
                                  Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rafael@kernel.org> - 2015-10-25 15:00 +0100
                                    Re: [GIT PULL] On-demand device probing Mark Brown <broonie@kernel.org> - 2015-10-26 02:20 +0100
                                    Re: [GIT PULL] On-demand device probing Michael Turquette <mturquette@baylibre.com> - 2015-10-26 12:00 +0100
                                      Re: [GIT PULL] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-10-26 14:00 +0100
                                      Re: [GIT PULL] On-demand device probing "Rafael J. Wysocki" <rafael@kernel.org> - 2015-10-27 00:40 +0100
                            Re: [GIT PULL] On-demand device probing "Andrew F. Davis" <afd@ti.com> - 2015-10-25 20:50 +0100
                          Re: [GIT PULL] On-demand device probing Geert Uytterhoeven <geert@linux-m68k.org> - 2015-10-24 20:00 +0200

Page 1 of 5  [1] 2 3 4 5  Next page →

#1250220 — Re: [GIT PULL] On-demand device probing

[Multipart message — attachments visible in raw view] — view raw

On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:

> I can't see adding calls like this all over the tree just to solve a
> bus-specific problem, you are adding of_* calls where they aren't
> needed, or wanted, at all.

This isn't bus specific, I'm not sure what makes you say that?

> What is the root-problem of your delay in device probing?  I read your
> last patch series and I can't seem to figure out what the issue is that
> this is solving in any "better" way from the existing deferred probing.

So, I don't actually have any platforms that are especially bothered by
this (at least not for my use cases) so there's a bit of educated
guessing going on here but there's two broad things I'm aware of.  

One is that regardless of the actual performance of the system when
deferred probe goes off it splats errors all over the console which
makes it look like something is going wrong even if everything is fine
in the end.  If lots of deferred probing happens then the volume gets
big too.  People find this distracting, noisy and ugly - it obscures
actual issues and trains people to ignore errors.  I do think this is a
reasonable concern and that it's worth trying to mitigate against
deferral for this reason alone.  We don't want to just ignore the errors
and not print anything either since if the resource doesn't appear the
user needs to know what is preventing the driver from instantiating so
they can try to fix it.

The other is that if you're printing to a serial console then that's not
an especially fast operation so if you're getting lots of messages being
printed simply physically outputting them takes measurable time.  I'm
not aware of any performance concerns outside of that, but like I say
I'm not affected by this myself in any great way.  Obviously this can be
configured but not having actual errors on the console isn't super
awesome either for systems that make use of the logging there and we
don't have a good way of telling what's from deferral and what's not.

[toc] | [next] | [standalone]

#1250223

On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> 
> > I can't see adding calls like this all over the tree just to solve a
> > bus-specific problem, you are adding of_* calls where they aren't
> > needed, or wanted, at all.
> 
> This isn't bus specific, I'm not sure what makes you say that?

You are making it bus-specific by putting these calls all over the tree
in different bus subsystems semi-randomly for all I can determine.

> > What is the root-problem of your delay in device probing?  I read your
> > last patch series and I can't seem to figure out what the issue is that
> > this is solving in any "better" way from the existing deferred probing.
> 
> So, I don't actually have any platforms that are especially bothered by
> this (at least not for my use cases) so there's a bit of educated
> guessing going on here but there's two broad things I'm aware of.  
> 
> One is that regardless of the actual performance of the system when
> deferred probe goes off it splats errors all over the console which
> makes it look like something is going wrong even if everything is fine
> in the end.  If lots of deferred probing happens then the volume gets
> big too.  People find this distracting, noisy and ugly - it obscures
> actual issues and trains people to ignore errors.  I do think this is a
> reasonable concern and that it's worth trying to mitigate against
> deferral for this reason alone.  We don't want to just ignore the errors
> and not print anything either since if the resource doesn't appear the
> user needs to know what is preventing the driver from instantiating so
> they can try to fix it.

This has come up many times, I have no objection to just turning that
message into a debug message that can be dynamically enabled for those
people wanting to debug their systems for boot time issues.

Please send a patch to do so.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250225

[Multipart message — attachments visible in raw view] — view raw

On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:

> > > I can't see adding calls like this all over the tree just to solve a
> > > bus-specific problem, you are adding of_* calls where they aren't
> > > needed, or wanted, at all.

> > This isn't bus specific, I'm not sure what makes you say that?

> You are making it bus-specific by putting these calls all over the tree
> in different bus subsystems semi-randomly for all I can determine.

Do you mean firmware rather than bus here?  I think that's the confusion
I have...

> > One is that regardless of the actual performance of the system when
> > deferred probe goes off it splats errors all over the console which
> > makes it look like something is going wrong even if everything is fine
> > in the end.  If lots of deferred probing happens then the volume gets
> > big too.  People find this distracting, noisy and ugly - it obscures
> > actual issues and trains people to ignore errors.  I do think this is a
> > reasonable concern and that it's worth trying to mitigate against
> > deferral for this reason alone.  We don't want to just ignore the errors
> > and not print anything either since if the resource doesn't appear the
> > user needs to know what is preventing the driver from instantiating so
> > they can try to fix it.

> This has come up many times, I have no objection to just turning that
> message into a debug message that can be dynamically enabled for those
> people wanting to debug their systems for boot time issues.

It's not just the driver core logging, it's also all the individual
drivers logging that they failed to get whatever resource since silently
failing is not a great user experience.  Many, hopefully most, of the
drivers don't actually have special handling for probe deferral since
half the beauty of probe deferral is that the subsystem supplying the
resource can just return -EPROBE_DEFER when it notices something is
missing but might appear and then the drivers will do the right thing so
long as they have error handling code that they really should have
anyway.

We'd need to have a special dev_err() that handled probe deferral
errors for drivers to use during probe or some other smarts in the
logging infrastructure.  Which isn't a totally horrible idea.

[toc] | [prev] | [next] | [standalone]

#1250524

[Multipart message — attachments visible in raw view] — view raw

On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> 
> > > > I can't see adding calls like this all over the tree just to solve a
> > > > bus-specific problem, you are adding of_* calls where they aren't
> > > > needed, or wanted, at all.
> 
> > > This isn't bus specific, I'm not sure what makes you say that?
> 
> > You are making it bus-specific by putting these calls all over the tree
> > in different bus subsystems semi-randomly for all I can determine.
> 
> Do you mean firmware rather than bus here?  I think that's the confusion
> I have...

Certainly, if it literally is adding of_* calls then that would seem to
be gratuitously firmware-specific. Nothing should be using those these
days; any new code should be using the generic device property APIs
(except in special cases).

-- 
dwmw2

[toc] | [prev] | [next] | [standalone]

#1250530

On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> > 
> > > > > I can't see adding calls like this all over the tree just to solve a
> > > > > bus-specific problem, you are adding of_* calls where they aren't
> > > > > needed, or wanted, at all.
> > 
> > > > This isn't bus specific, I'm not sure what makes you say that?
> > 
> > > You are making it bus-specific by putting these calls all over the tree
> > > in different bus subsystems semi-randomly for all I can determine.
> > 
> > Do you mean firmware rather than bus here?  I think that's the confusion
> > I have...
> 
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).

I asked Linus Walleij about that with the fwnode_get_named_gpiod() stuff,
and Linus didn't seem to know how this should be used.

It doesn't help that dev->fwnode is not initialised, but dev->of_node
is.  Are we supposed to grope around in dev->of_node for the embedded
fwnode instead of using dev->fwnode?

At the moment, at least to me, fwnode looks like some kind of
experimental half-baked thing rather than a real usable solution.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250567

[Multipart message — attachments visible in raw view] — view raw

On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:

> > Do you mean firmware rather than bus here?  I think that's the confusion
> > I have...

> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).

It's not entirely clear to me that we should be moving to fwnode_
wholesale yet - the last advice was to hold off for a little while which
makes sense given that the ACPI community still doesn't seem to have
worked out what it wants to do here and how.  The x86 embedded people
are all gung ho but it's less clear that anyone else wants to use _DSD
in quite the same way (I know of some efforts to use _DSD separately to
the DT compatibility stuff) and there are some vendors who definitely do
have completely different binding schemes for ACPI and DT and therefore
specifically care which is in use.

It would really help if ACPI could get their binding review process in
place, and if we do want to actually start converting everything to
fwnode_ we need to start communicating that actively since otherwise
people can't really be expected to know.

[toc] | [prev] | [next] | [standalone]

#1250615

On Mon, Oct 19, 2015 at 4:44 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
>> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>>
>> > > > I can't see adding calls like this all over the tree just to solve a
>> > > > bus-specific problem, you are adding of_* calls where they aren't
>> > > > needed, or wanted, at all.
>>
>> > > This isn't bus specific, I'm not sure what makes you say that?
>>
>> > You are making it bus-specific by putting these calls all over the tree
>> > in different bus subsystems semi-randomly for all I can determine.
>>
>> Do you mean firmware rather than bus here?  I think that's the confusion
>> I have...
>
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).

See version 2 of the series[1] which did that. It became obvious that
was pointless because the call paths ended up looking like this:

Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
of_probe_device

Rob

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361137.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250623

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2015-10-19 at 07:35 -0500, Rob Herring wrote:
> 
> > Certainly, if it literally is adding of_* calls then that would seem to
> > be gratuitously firmware-specific. Nothing should be using those these
> > days; any new code should be using the generic device property APIs
> > (except in special cases).
> 
> See version 2 of the series[1] which did that. It became obvious that
> was pointless because the call paths ended up looking like this:
> 
> Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
> of_probe_device

You link to a thread which says that "AT LEAST CURRENTLY, the calling
locations [the 'DT look-up code' you mention above] are DT specific
functions anyway.

But the point I'm making is that we are working towards *fixing* that,
and *not* using DT-specific code in places where we should be using the
generic APIs.

Sure, Russell is probably right that there are some places where the
generic APIs need fixing because they don't quite cover all use cases
yet.

And Mark is (unfortunately) right that some people are inventing new
bindings *purely* for ACPI which are different to the DT bindings for
the same device. But still, in those cases you'll theoretically be able
to see the *same* device represented under ACPI with *either* its new
ACPI HID and the ACPI-specific bindings, *or* as a PRP0001 with the DT
bindings. And this was always possible even with just DT — you could
have two incompatible bindings for the *same* hardware, with different
drivers. It was just a bad thing. And still is when one is ACPI and one
is DT, in my opinion.

None of that really negates that fact that we are *working* on cleaning
these code paths up to be firmware-agnostic, and the fact that we
haven't got to this one *yet* isn't necessarily a good reason to make
it *worse* by adding new firmware-specificity to it.

-- 
dwmw2

[toc] | [prev] | [next] | [standalone]

#1250771

[Multipart message — attachments visible in raw view] — view raw

On Mon, Oct 19, 2015 at 01:47:50PM +0100, David Woodhouse wrote:
> On Mon, 2015-10-19 at 07:35 -0500, Rob Herring wrote:

> > See version 2 of the series[1] which did that. It became obvious that
> > was pointless because the call paths ended up looking like this:

> > Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
> > of_probe_device

> You link to a thread which says that "AT LEAST CURRENTLY, the calling
> locations [the 'DT look-up code' you mention above] are DT specific
> functions anyway.

> But the point I'm making is that we are working towards *fixing* that,
> and *not* using DT-specific code in places where we should be using the
> generic APIs.

What is the plan for fixing things here?  It's not obvious (at least to
me) that we don't want to have the subsystems having knowledge of how
they are bound to a specific firmware which is what you seem to imply
here.  That seems like it's going to fall down since the different
firmware interfaces do have quite different ideas about how things fit
together at a system level and different compatibility needs which do
suggest that just trying to do a direct mapping from DT into ACPI may
well not make people happy but it sounds like that's the intention.

When it gets to drivers the situation is much more clear since it's
normally just simple properties, it's generally a bit more worrying if
drivers are needing to directly interact with cross-device linkage.
This is all subsystem level code though.

> None of that really negates that fact that we are *working* on cleaning
> these code paths up to be firmware-agnostic, and the fact that we
> haven't got to this one *yet* isn't necessarily a good reason to make
> it *worse* by adding new firmware-specificity to it.

It seems like we're going to have to refactor these bits of code when
they get generalised anyway so I'm not sure that the additional cost
here is that big.

[toc] | [prev] | [next] | [standalone]

#1250853

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
> > But the point I'm making is that we are working towards *fixing* that,
> > and *not* using DT-specific code in places where we should be using the
> > generic APIs.
> 
> What is the plan for fixing things here?  It's not obvious (at least to
> me) that we don't want to have the subsystems having knowledge of how
> they are bound to a specific firmware which is what you seem to imply
> here. 

I don't know that there *is* a coherent plan here to address it all.

Certainly, we *will* need subsystems to have firmware-specific
knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
describe GPIO, and properties which reference GPIO pins are intended to
work through that — while in DT, properties which reference GPIO pins
will have different contents. They'll be compatible at the driver
level, in the sense that there's a call to get a given GPIO given the
property name, but the subsystems *will* be doing different things
behind the scenes.

My plan, such as it is, is to go through the leaf-node drivers which
almost definitely *should* be firmware-agnostic, and convert those. And
then take stock of what we have left, and work out what, if anything,
still needs to be done.

> It seems like we're going to have to refactor these bits of code when
> they get generalised anyway so I'm not sure that the additional cost
> here is that big.

That's an acceptable answer — "we're adding legacy code here but we
know it's going to be refactored anyway". If that's true, all it takes
is a note in the commit comment to that effect. That's different from
having not thought about it :)

-- 
dwmw2

[toc] | [prev] | [next] | [standalone]

#1250860

On Mon, Oct 19, 2015 at 04:29:40PM +0100, David Woodhouse wrote:
> I don't know that there *is* a coherent plan here to address it all.
> 
> Certainly, we *will* need subsystems to have firmware-specific
> knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> describe GPIO, and properties which reference GPIO pins are intended to
> work through that — while in DT, properties which reference GPIO pins
> will have different contents. They'll be compatible at the driver
> level, in the sense that there's a call to get a given GPIO given the
> property name, but the subsystems *will* be doing different things
> behind the scenes.

It's a bit ironic that you've chosen GPIO as an example there.  The
"new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
gpio descriptor.  There's no of_* method.

I'd like to use the gpiod_* stuff, but I feel that my options are
rather limited: either use fwnode_get_named_gpiod() with
&dev->of_node->fwnode, which seems like a hack by going underneath
the covers of how fwnode is (partially) implemented with DT, or by
using of_get_named_gpio() and the converting the gpio number to a
descriptor via gpio_to_desc().  Both feel very hacky.

If ACPI already handles GPIOs internally, then I'm left wondering
why GPIO descriptor stuff went down the fwnode route at all - it
seems rather pointless in this case, and it seems to make the use
of the gpiod* interfaces where we _do_ need to use it (DT) harder
and more hacky.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250989

Hello,

On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:
> It's a bit ironic that you've chosen GPIO as an example there.  The
> "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
> gpio descriptor.  There's no of_* method.

Without following all that fwnode discussion:
gpiod_get et al. should work for you here, doesn't it? It just takes a
struct device * and I'm happy with it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251003

On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:
> > It's a bit ironic that you've chosen GPIO as an example there.  The
> > "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
> > gpio descriptor.  There's no of_* method.
> 
> Without following all that fwnode discussion:
> gpiod_get et al. should work for you here, doesn't it? It just takes a
> struct device * and I'm happy with it.

What if you don't have a struct device?  I had that problem recently
when modifying the mvebu PCIe code.  The 'struct device' node doesn't
contain the GPIOs, it's the PCIe controller.  Individual ports on the
controller are described in DT as sub-nodes, and the sub-nodes can
have a GPIO for card reset purposes.  These sub-nodes don't have a
struct device.

Right now, I'm having to do this to work around this issue:

        reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags);
        if (reset_gpio == -EPROBE_DEFER) {
                ret = reset_gpio;
                goto err;
        }

        if (gpio_is_valid(reset_gpio)) {
                unsigned long gpio_flags;

                port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset",
                                                  port->name);
                if (!port->reset_name) {
                        ret = -ENOMEM;
                        goto err;
                }

                if (flags & OF_GPIO_ACTIVE_LOW) {
                        dev_info(dev, "%s: reset gpio is active low\n",
                                 of_node_full_name(child));
                        gpio_flags = GPIOF_ACTIVE_LOW |
                                     GPIOF_OUT_INIT_LOW;
                } else {
                        gpio_flags = GPIOF_OUT_INIT_HIGH;
                }

                ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags,
                                            port->reset_name);
                if (ret) {
                        if (ret == -EPROBE_DEFER)
                                goto err;
                        goto skip;
                }

                port->reset_gpio = gpio_to_desc(reset_gpio);
        }

Not nice, is it?  Not nice to have that in lots of drivers either.

However, switching to use any of_* or fwnode_* thing also carries with
it another problem: you can't control the name appearing in the
allocation, so you end up with a bunch of GPIOs requested with a "reset"
name - meaning you lose any identification of which port the GPIO was
bound to.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251156

On Tue, Oct 20, 2015 at 3:39 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote:
>> Hello,
>>
>> On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:
>> > It's a bit ironic that you've chosen GPIO as an example there.  The
>> > "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
>> > gpio descriptor.  There's no of_* method.
>>
>> Without following all that fwnode discussion:
>> gpiod_get et al. should work for you here, doesn't it? It just takes a
>> struct device * and I'm happy with it.
>
> What if you don't have a struct device?  I had that problem recently
> when modifying the mvebu PCIe code.  The 'struct device' node doesn't
> contain the GPIOs, it's the PCIe controller.  Individual ports on the
> controller are described in DT as sub-nodes, and the sub-nodes can
> have a GPIO for card reset purposes.  These sub-nodes don't have a
> struct device.
>
> Right now, I'm having to do this to work around this issue:
>
>         reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags);
>         if (reset_gpio == -EPROBE_DEFER) {
>                 ret = reset_gpio;
>                 goto err;
>         }
>
>         if (gpio_is_valid(reset_gpio)) {
>                 unsigned long gpio_flags;
>
>                 port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset",
>                                                   port->name);
>                 if (!port->reset_name) {
>                         ret = -ENOMEM;
>                         goto err;
>                 }
>
>                 if (flags & OF_GPIO_ACTIVE_LOW) {
>                         dev_info(dev, "%s: reset gpio is active low\n",
>                                  of_node_full_name(child));
>                         gpio_flags = GPIOF_ACTIVE_LOW |
>                                      GPIOF_OUT_INIT_LOW;
>                 } else {
>                         gpio_flags = GPIOF_OUT_INIT_HIGH;
>                 }
>
>                 ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags,
>                                             port->reset_name);
>                 if (ret) {
>                         if (ret == -EPROBE_DEFER)
>                                 goto err;
>                         goto skip;
>                 }
>
>                 port->reset_gpio = gpio_to_desc(reset_gpio);
>         }
>
> Not nice, is it?  Not nice to have that in lots of drivers either.
>
> However, switching to use any of_* or fwnode_* thing also carries with
> it another problem: you can't control the name appearing in the
> allocation, so you end up with a bunch of GPIOs requested with a "reset"
> name - meaning you lose any identification of which port the GPIO was
> bound to.

There are a few holes in the gpiod API. I see two solutions here:

1) extend devm_get_gpiod_from_child() to take an optional name argument
2) add a function to explicitly change a GPIO's name

2) seems to be the most generic solution, would that do the trick?

(sorry for the off-topic)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251323 — Re: gpiod API considerations [Was: [GIT PULL] On-demand device probing]

Hello,

[trimming list of recipients considerably because of changed topic]

On Tue, Oct 20, 2015 at 08:47:21AM +0900, Alexandre Courbot wrote:
> On Tue, Oct 20, 2015 at 3:39 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote:
> >> On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:
> >> > It's a bit ironic that you've chosen GPIO as an example there.  The
> >> > "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
> >> > gpio descriptor.  There's no of_* method.
> >>
> >> Without following all that fwnode discussion:
> >> gpiod_get et al. should work for you here, doesn't it? It just takes a
> >> struct device * and I'm happy with it.
> >
> > What if you don't have a struct device?  I had that problem recently
> > when modifying the mvebu PCIe code.  The 'struct device' node doesn't
> > contain the GPIOs, it's the PCIe controller.  Individual ports on the
> > controller are described in DT as sub-nodes, and the sub-nodes can
> > have a GPIO for card reset purposes.  These sub-nodes don't have a
> > struct device.
> >
> > Right now, I'm having to do this to work around this issue:
> >
> >         reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags);
> >         if (reset_gpio == -EPROBE_DEFER) {
> >                 ret = reset_gpio;
> >                 goto err;
> >         }
> >
> >         if (gpio_is_valid(reset_gpio)) {
> >                 unsigned long gpio_flags;
> >
> >                 port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset",
> >                                                   port->name);
> >                 if (!port->reset_name) {
> >                         ret = -ENOMEM;
> >                         goto err;
> >                 }
> >
> >                 if (flags & OF_GPIO_ACTIVE_LOW) {
> >                         dev_info(dev, "%s: reset gpio is active low\n",
> >                                  of_node_full_name(child));
> >                         gpio_flags = GPIOF_ACTIVE_LOW |
> >                                      GPIOF_OUT_INIT_LOW;
> >                 } else {
> >                         gpio_flags = GPIOF_OUT_INIT_HIGH;
> >                 }
> >
> >                 ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags,
> >                                             port->reset_name);
> >                 if (ret) {
> >                         if (ret == -EPROBE_DEFER)
> >                                 goto err;
> >                         goto skip;
> >                 }
> >
> >                 port->reset_gpio = gpio_to_desc(reset_gpio);
> >         }
> >
> > Not nice, is it?  Not nice to have that in lots of drivers either.
> >
> > However, switching to use any of_* or fwnode_* thing also carries with
> > it another problem: you can't control the name appearing in the
> > allocation, so you end up with a bunch of GPIOs requested with a "reset"
> > name - meaning you lose any identification of which port the GPIO was
> > bound to.
> 
> There are a few holes in the gpiod API. I see two solutions here:
> 
> 1) extend devm_get_gpiod_from_child() to take an optional name argument
> 2) add a function to explicitly change a GPIO's name
> 
> 2) seems to be the most generic solution, would that do the trick?

I would prefer 1) without "optional". A third alternative is to add at
least dev_name(dev) and maybe index to the name where applicable. Also
note that gpiod_request is called with label=NULL (in
fwnode_get_named_gpiod which is used in devm_get_gpiod_from_child), so
/sys/kernel/debug/gpio doesn't even contain "reset". I only see question
marks (using v4.3-rc5).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251595

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2015-10-19 at 16:43 +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 19, 2015 at 04:29:40PM +0100, David Woodhouse wrote:
> > I don't know that there *is* a coherent plan here to address it
> > all.
> > 
> > Certainly, we *will* need subsystems to have firmware-specific
> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way
> > to
> > describe GPIO, and properties which reference GPIO pins are
> > intended to
> > work through that — while in DT, properties which reference GPIO
> > pins
> > will have different contents. They'll be compatible at the driver
> > level, in the sense that there's a call to get a given GPIO given
> > the
> > property name, but the subsystems *will* be doing different things
> > behind the scenes.
> 
> It's a bit ironic that you've chosen GPIO as an example there.  The
> "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
> gpio descriptor.  There's no of_* method.

I think that part is already being worked on, but...

> If ACPI already handles GPIOs internally, then I'm left wondering
> why GPIO descriptor stuff went down the fwnode route at all - it
> seems rather pointless in this case, 

ACPI already had a way for a given device to say that it uses certain
other GPIOs. But until we had device properties in ACPI, it could say
*what* it used them for. So sure, we could say that we used GPIO#15
from <this> controller and GPIOs #27 and #31 from <that> controller.
But there was no way to say that the former was the shotdown pin and
the latter was the reset pin.

While a GPIO property in DT will contain a phandle and basically be a
complete reference to find the pin you're after, the same property
represented in ACPI will just be an index into the resources that ACPI
could already refer to.

So referring to the example in Documentation/acpi/gpio-properties.txt:

      Name (_CRS, ResourceTemplate ()
      {
          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {15}
          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {27, 31}
      })

That part, ACPI already had. But..

      Name (_DSD, Package ()
      {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package ()
          {
              Package () {"reset-gpio", Package() {^BTH, 1, 1, 0 }},
              Package () {"shutdown-gpio", Package() {^BTH, 0, 0, 0 }},
          }
      })

...this part is new, and allows us the full flexibility of device
properties. And the appropriate gpiod_get* function is supposed to
transparently work on either DT or ACPI.


-- 
dwmw2

[toc] | [prev] | [next] | [standalone]

#1250866

On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
>> > But the point I'm making is that we are working towards *fixing* that,
>> > and *not* using DT-specific code in places where we should be using the
>> > generic APIs.
>>
>> What is the plan for fixing things here?  It's not obvious (at least to
>> me) that we don't want to have the subsystems having knowledge of how
>> they are bound to a specific firmware which is what you seem to imply
>> here.
>
> I don't know that there *is* a coherent plan here to address it all.
>
> Certainly, we *will* need subsystems to have firmware-specific
> knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> describe GPIO, and properties which reference GPIO pins are intended to
> work through that — while in DT, properties which reference GPIO pins
> will have different contents. They'll be compatible at the driver
> level, in the sense that there's a call to get a given GPIO given the
> property name, but the subsystems *will* be doing different things
> behind the scenes.
>
> My plan, such as it is, is to go through the leaf-node drivers which
> almost definitely *should* be firmware-agnostic, and convert those. And
> then take stock of what we have left, and work out what, if anything,
> still needs to be done.

Many cases are already agnostic in the drivers in terms of the *_get()
functions. Some are DT specific, but probably because those subsystems
are new and DT only. In any case, I don't think these 1 line changes
do anything to make doing conversions here harder.

>> It seems like we're going to have to refactor these bits of code when
>> they get generalised anyway so I'm not sure that the additional cost
>> here is that big.
>
> That's an acceptable answer — "we're adding legacy code here but we
> know it's going to be refactored anyway". If that's true, all it takes
> is a note in the commit comment to that effect. That's different from
> having not thought about it :)

Considering at one point we did create a fwnode based API, we did
think about it. Plus there was little input from ACPI folks as to
whether the change was even useful for ACPI case. In any case, we're
talking about adding 1 line.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251088

On Monday, October 19, 2015 10:58:25 AM Rob Herring wrote:
> On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> > On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
> >> > But the point I'm making is that we are working towards *fixing* that,
> >> > and *not* using DT-specific code in places where we should be using the
> >> > generic APIs.
> >>
> >> What is the plan for fixing things here?  It's not obvious (at least to
> >> me) that we don't want to have the subsystems having knowledge of how
> >> they are bound to a specific firmware which is what you seem to imply
> >> here.
> >
> > I don't know that there *is* a coherent plan here to address it all.
> >
> > Certainly, we *will* need subsystems to have firmware-specific
> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> > describe GPIO, and properties which reference GPIO pins are intended to
> > work through that — while in DT, properties which reference GPIO pins
> > will have different contents. They'll be compatible at the driver
> > level, in the sense that there's a call to get a given GPIO given the
> > property name, but the subsystems *will* be doing different things
> > behind the scenes.
> >
> > My plan, such as it is, is to go through the leaf-node drivers which
> > almost definitely *should* be firmware-agnostic, and convert those. And
> > then take stock of what we have left, and work out what, if anything,
> > still needs to be done.
> 
> Many cases are already agnostic in the drivers in terms of the *_get()
> functions. Some are DT specific, but probably because those subsystems
> are new and DT only. In any case, I don't think these 1 line changes
> do anything to make doing conversions here harder.
> 
> >> It seems like we're going to have to refactor these bits of code when
> >> they get generalised anyway so I'm not sure that the additional cost
> >> here is that big.
> >
> > That's an acceptable answer — "we're adding legacy code here but we
> > know it's going to be refactored anyway". If that's true, all it takes
> > is a note in the commit comment to that effect. That's different from
> > having not thought about it :)
> 
> Considering at one point we did create a fwnode based API, we did
> think about it. Plus there was little input from ACPI folks as to
> whether the change was even useful for ACPI case.

Well, sorry, but who was asking whom, specifically?

The underlying problem is present in ACPI too and we don't really have a good
solution for it.  We might benefit from a common one if it existed.

> In any case, we're talking about adding 1 line.

But also about making the driver core slighly OF-centric.

Sure, we need OF-specific code and ACPI-specific code wherever different
handling is required, but doing that at the driver core level seems to be
a bit of a stretch to me.

Please note that we don't really have ACPI-specific calls in the driver core,
although we might have added them long ago even before the OF stuff appeared
in the kernel for the first time.  We didn't do that, (among other things)
because we didn't want that particular firmware interface to appear special
in any way and I'm not really sure why it is now OK to make OF look special
instead.

If it is trivial to avoid that (and you seem to be arguing that it is), why
do we have to do it?

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251144

On Mon, Oct 19, 2015 at 4:40 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Monday, October 19, 2015 10:58:25 AM Rob Herring wrote:
>> On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>> > On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
>> >> > But the point I'm making is that we are working towards *fixing* that,
>> >> > and *not* using DT-specific code in places where we should be using the
>> >> > generic APIs.
>> >>
>> >> What is the plan for fixing things here?  It's not obvious (at least to
>> >> me) that we don't want to have the subsystems having knowledge of how
>> >> they are bound to a specific firmware which is what you seem to imply
>> >> here.
>> >
>> > I don't know that there *is* a coherent plan here to address it all.
>> >
>> > Certainly, we *will* need subsystems to have firmware-specific
>> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
>> > describe GPIO, and properties which reference GPIO pins are intended to
>> > work through that — while in DT, properties which reference GPIO pins
>> > will have different contents. They'll be compatible at the driver
>> > level, in the sense that there's a call to get a given GPIO given the
>> > property name, but the subsystems *will* be doing different things
>> > behind the scenes.
>> >
>> > My plan, such as it is, is to go through the leaf-node drivers which
>> > almost definitely *should* be firmware-agnostic, and convert those. And
>> > then take stock of what we have left, and work out what, if anything,
>> > still needs to be done.
>>
>> Many cases are already agnostic in the drivers in terms of the *_get()
>> functions. Some are DT specific, but probably because those subsystems
>> are new and DT only. In any case, I don't think these 1 line changes
>> do anything to make doing conversions here harder.
>>
>> >> It seems like we're going to have to refactor these bits of code when
>> >> they get generalised anyway so I'm not sure that the additional cost
>> >> here is that big.
>> >
>> > That's an acceptable answer — "we're adding legacy code here but we
>> > know it's going to be refactored anyway". If that's true, all it takes
>> > is a note in the commit comment to that effect. That's different from
>> > having not thought about it :)
>>
>> Considering at one point we did create a fwnode based API, we did
>> think about it. Plus there was little input from ACPI folks as to
>> whether the change was even useful for ACPI case.
>
> Well, sorry, but who was asking whom, specifically?

You and linux-acpi have been copied on v2 and later of the entire
series I think.

> The underlying problem is present in ACPI too and we don't really have a good
> solution for it.  We might benefit from a common one if it existed.

The problem for DT is we don't generically know what are the
dependencies at a core level. We could know some or most dependencies
if phandles (links to other nodes) were typed, but they are not. If
the core had this information, we could simply control the device
creation to order probing. Instead, this information is encoded into
the bindings and binding parsing resides in the subsystems. That
parsing happens during probe of the client side and is done by the
subsystems (for common bindings). Since we already do the parsing at
this point, it is a convenient place to trigger the probe of the
dependency. Is ACPI going to be similar in this regard?

Fundamentally, it is a question of probe devices when their
dependencies are present or drivers ensure their dependencies are
ready. IIRC, init systems went thru a similar debate for service
dependencies.

>> In any case, we're talking about adding 1 line.
>
> But also about making the driver core slighly OF-centric.

How so? The one line is in DT binding parsing code in subsystems, not
driver core. The driver core change is we add every device (that
happened to be created by DT) to the deferred probe list, so they
don't probe right away.


> Sure, we need OF-specific code and ACPI-specific code wherever different
> handling is required, but doing that at the driver core level seems to be
> a bit of a stretch to me.
>
> Please note that we don't really have ACPI-specific calls in the driver core,
> although we might have added them long ago even before the OF stuff appeared
> in the kernel for the first time.  We didn't do that, (among other things)
> because we didn't want that particular firmware interface to appear special
> in any way and I'm not really sure why it is now OK to make OF look special
> instead.

I don't think DT is special and we avoid DT specific core changes as
much as possible. I think the difference is DT uses platform_device
and ACPI does not. It used to be separate, but got merged together
primarily to support the plethora of existing drivers. Anyway, that is
all outside of anything in this series.

> If it is trivial to avoid that (and you seem to be arguing that it is), why
> do we have to do it?

Sorry, I don't follow what "that" or "it" is.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251336

On Monday, October 19, 2015 05:58:40 PM Rob Herring wrote:
> On Mon, Oct 19, 2015 at 4:40 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Monday, October 19, 2015 10:58:25 AM Rob Herring wrote:
> >> On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> >> > On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
> >> >> > But the point I'm making is that we are working towards *fixing* that,
> >> >> > and *not* using DT-specific code in places where we should be using the
> >> >> > generic APIs.
> >> >>
> >> >> What is the plan for fixing things here?  It's not obvious (at least to
> >> >> me) that we don't want to have the subsystems having knowledge of how
> >> >> they are bound to a specific firmware which is what you seem to imply
> >> >> here.
> >> >
> >> > I don't know that there *is* a coherent plan here to address it all.
> >> >
> >> > Certainly, we *will* need subsystems to have firmware-specific
> >> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> >> > describe GPIO, and properties which reference GPIO pins are intended to
> >> > work through that — while in DT, properties which reference GPIO pins
> >> > will have different contents. They'll be compatible at the driver
> >> > level, in the sense that there's a call to get a given GPIO given the
> >> > property name, but the subsystems *will* be doing different things
> >> > behind the scenes.
> >> >
> >> > My plan, such as it is, is to go through the leaf-node drivers which
> >> > almost definitely *should* be firmware-agnostic, and convert those. And
> >> > then take stock of what we have left, and work out what, if anything,
> >> > still needs to be done.
> >>
> >> Many cases are already agnostic in the drivers in terms of the *_get()
> >> functions. Some are DT specific, but probably because those subsystems
> >> are new and DT only. In any case, I don't think these 1 line changes
> >> do anything to make doing conversions here harder.
> >>
> >> >> It seems like we're going to have to refactor these bits of code when
> >> >> they get generalised anyway so I'm not sure that the additional cost
> >> >> here is that big.
> >> >
> >> > That's an acceptable answer — "we're adding legacy code here but we
> >> > know it's going to be refactored anyway". If that's true, all it takes
> >> > is a note in the commit comment to that effect. That's different from
> >> > having not thought about it :)
> >>
> >> Considering at one point we did create a fwnode based API, we did
> >> think about it. Plus there was little input from ACPI folks as to
> >> whether the change was even useful for ACPI case.
> >
> > Well, sorry, but who was asking whom, specifically?
> 
> You and linux-acpi have been copied on v2 and later of the entire
> series I think.

Yes, but it wasn't like a direct request, say "We need your input, so can you
please have a look and BTW we want this in 4.4, so please do it ASAP".  In
which case I'd prioritize that before other things I needed to take care of.

> > The underlying problem is present in ACPI too and we don't really have a good
> > solution for it.  We might benefit from a common one if it existed.
> 
> The problem for DT is we don't generically know what are the
> dependencies at a core level. We could know some or most dependencies
> if phandles (links to other nodes) were typed, but they are not. If
> the core had this information, we could simply control the device
> creation to order probing. Instead, this information is encoded into
> the bindings and binding parsing resides in the subsystems. That
> parsing happens during probe of the client side and is done by the
> subsystems (for common bindings). Since we already do the parsing at
> this point, it is a convenient place to trigger the probe of the
> dependency. Is ACPI going to be similar in this regard?

It is similar in some ways.  For example, if a device's functionality depends
on an I2C resource (connection), the core doesn't know that at the device
creation time at least in some cases.  Same for GPIO, SPI, DMA engines etc.

There is a _DEP object in ACPI that can be used by firmware to tell the OS
about those dependencies, but there's no way in the driver core to use that
information anyway today.

> Fundamentally, it is a question of probe devices when their
> dependencies are present or drivers ensure their dependencies are
> ready. IIRC, init systems went thru a similar debate for service
> dependencies.

The probe ordering is not the entire picture, though.

Even if you get the probe ordering right, the problem is going to show up in
multiple other places: system suspend/resume, runtime PM, system shutdown,
unbinding of drivers.  In all of those cases it is necessary to handle things
in a specific order if there is a dependency.

> >> In any case, we're talking about adding 1 line.
> >
> > But also about making the driver core slighly OF-centric.
> 
> How so? The one line is in DT binding parsing code in subsystems, not
> driver core. The driver core change is we add every device (that
> happened to be created by DT) to the deferred probe list, so they
> don't probe right away.

The "that happened to be created by DT" part is of concern here.  What is there
that makes DT special in that respect?  Why shouldn't that be applicable to
devices created by the ACPI core, for example, or by a board file or something
else?

> > Sure, we need OF-specific code and ACPI-specific code wherever different
> > handling is required, but doing that at the driver core level seems to be
> > a bit of a stretch to me.
> >
> > Please note that we don't really have ACPI-specific calls in the driver core,
> > although we might have added them long ago even before the OF stuff appeared
> > in the kernel for the first time.  We didn't do that, (among other things)
> > because we didn't want that particular firmware interface to appear special
> > in any way and I'm not really sure why it is now OK to make OF look special
> > instead.
> 
> I don't think DT is special and we avoid DT specific core changes as
> much as possible. I think the difference is DT uses platform_device
> and ACPI does not.

ACPI uses platform devices too.  In fact, ACPI device objects are enumerated as
platform devices by default now.

Or do you means something else here?

> It used to be separate, but got merged together primarily to support the
> plethora of existing drivers. Anyway, that is all outside of anything in this
> series.

It explains the context of the series, so it is useful to talk about IMO.

> 
> > If it is trivial to avoid that (and you seem to be arguing that it is), why
> > do we have to do it?
> 
> Sorry, I don't follow what "that" or "it" is.

OK, so maybe I misunderstood you, sorry about that.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 5  [1] 2 3 4 5  Next page →

Back to top | Article view | linux.kernel

csiph-web