* Re: [GIT PULL] On-demand device probing
From: Alexandre Courbot @ 2015-10-19 23:47 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Uwe Kleine-König, David Woodhouse, Mark Brown, Rob Herring,
Greg Kroah-Hartman, Tomeu Vizoso, Michael Turquette, Stephen Boyd,
Vinod Koul, Dan Williams, Linus Walleij, Thierry Reding,
David Airlie, Terje Bergström, Stephen Warren, Wolfram Sang,
Frank Rowand, Grant Likely, Kishon Vijay Abraham I,
Sebastian Reichel, Dmitry Eremin-Solenikov
In-Reply-To: <20151019183905.GS32532@n2100.arm.linux.org.uk>
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)
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Rob Herring @ 2015-10-19 22:58 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: David Woodhouse, Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso,
Russell King, Michael Turquette, Stephen Boyd, Vinod Koul,
Dan Williams, Linus Walleij, Alexandre Courbot, Thierry Reding,
David Airlie, Terje Bergström, Stephen Warren, Wolfram Sang,
Frank Rowand, Grant Likely, Kishon Vijay Abraham I,
Sebastian Reichel, Dmitry Eremin-Solenikov
In-Reply-To: <1603415.VWFZ8V0WMC@vostro.rjw.lan>
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
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Rafael J. Wysocki @ 2015-10-19 21:40 UTC (permalink / raw)
To: Rob Herring
Cc: David Woodhouse, Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso,
Russell King, Michael Turquette, Stephen Boyd, Vinod Koul,
Dan Williams, Linus Walleij, Alexandre Courbot, Thierry Reding,
David Airlie, Terje Bergström, Stephen Warren, Wolfram Sang,
Frank Rowand, Grant Likely, Kishon Vijay Abraham I,
Sebastian Reichel, Dmitry Eremin-Solenikov
In-Reply-To: <CAL_JsqLwvc1H9kQnWs=mA8P4Mq4izmSSeFU6KRtkExBkzF9ABg@mail.gmail.com>
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
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 18:39 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: David Woodhouse, Mark Brown, Rob Herring, Greg Kroah-Hartman,
Tomeu Vizoso, Michael Turquette, Stephen Boyd, Vinod Koul,
Dan Williams, Linus Walleij, Alexandre Courbot, Thierry Reding,
David Airlie, Terje Bergström, Stephen Warren, Wolfram Sang,
Frank Rowand, Grant Likely, Kishon Vijay Abraham I,
Sebastian Reichel, Dmitry Eremin-Solenikov, Liam Girdwood
In-Reply-To: <20151019182744.GD4931@pengutronix.de>
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.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Uwe Kleine-König @ 2015-10-19 18:27 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: David Woodhouse, Mark Brown, Rob Herring, Greg Kroah-Hartman,
Tomeu Vizoso, Michael Turquette, Stephen Boyd, Vinod Koul,
Dan Williams, Linus Walleij, Alexandre Courbot, Thierry Reding,
David Airlie, Terje Bergström, Stephen Warren, Wolfram Sang,
Frank Rowand, Grant Likely, Kishon Vijay Abraham I,
Sebastian Reichel, Dmitry Eremin-Solenikov, Liam Girdwood
In-Reply-To: <20151019154324.GN32532@n2100.arm.linux.org.uk>
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/ |
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Olof Johansson @ 2015-10-19 16:52 UTC (permalink / raw)
To: Rob Herring
Cc: Tomeu Vizoso, Russell King, Greg Kroah-Hartman, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood
In-Reply-To: <CAL_Jsq+34sRXe0HWCKNceQfLj=WfDV4-soynumvdnH_nwuj+9g@mail.gmail.com>
On Sat, Oct 17, 2015 at 8:19 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Oct 16, 2015 at 4:23 PM, Olof Johansson <olof@lixom.net> wrote:
>> Hi,
>>
>> I've bisected boot failures in next-20151016 down to patches in this branch:
>>
>> On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
>> <tomeu.vizoso@collabora.com> wrote:
>>> Tomeu Vizoso (20):
>>> driver core: handle -EPROBE_DEFER from bus_type.match()
>>
>> The machine it happened on was OMAP5UEVM:
>>
>> http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html
>
> So this one is because the MMC node numbering changed. I don't know
> how to fix that other than with aliases, but that doesn't solve
> backwards compatibility.
Yep, aliases will take care of it in this case. This is where -next
fills a great purpose, we can make sure we get those aliases added in
before the patches go in.
>> But I've also seen it on tegra2, that one bisected down to:
>>
>>> regulator: core: Probe regulators on demand
>>
>> http://arm-soc.lixom.net/bootlogs/next/next-20151016/seaboard-arm-multi_v7_defconfig.html
>
> This one you need a rootwait I think. The MMC scanning is not
> guaranteed to be done before the rootfs mounting AFAIK. There may be
> other problems, but we can't see them since it panics.
Embarrassing, I almost always do this and I'm surprised this machine
has been this stable without it.
-Olof
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 16:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Tomeu Vizoso, Mark Brown, Greg Kroah-Hartman, Rob Herring,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood
In-Reply-To: <CAMuHMdVv=ieYM-Nehp0qV8CEggHEeJ22j-6i_N09rRA_U=2ePw@mail.gmail.com>
On Mon, Oct 19, 2015 at 06:21:40PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
>
> On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> >> > What you can do is print those devices which have failed to probe at
> >> > late_initcall() time - possibly augmenting that with reports from
> >> > subsystems showing what resources are not available, but that's only
> >> > a guide, because of the "it might or might not be in a kernel module"
> >> > problem.
> >>
> >> Well, adding those reports would give you a changelog similar to the
> >> one in this series...
> >
> > I'm not sure about that, because what I was thinking of is adding
> > a flag which would be set at late_initcall() time prior to running
> > a final round of deferred device probing.
>
> Which round is the final round?
I said "a" not "the". Maybe I should've said "one last round of deferred
probing before entering userspace".
> That's the one which didn't manage to bind any new devices to drivers,
> which is something you only know _after_ the round has been run.
>
> So I think we need one extra round to handle this.
Yes - because the idea is that we do everything we do today without
printing anything about deferred probes. We then set a flag which
indicates we should report defers, and then we trigger another round
of probes.
If everything probed successfully, the deferred probe list will be
empty and nothing will be seen. Otherwise, we should end up with a
report of all the devices that weren't able to be bound to their
drivers due to -EPROBE_DEFER.
>
> > This flag would then be used in a deferred_warn() printk function
> > which would normally be silent, but when this flag is set, it would
> > print the reason for the deferral - and this would replace (or be
> > added) to the subsystems and drivers which return -EPROBE_DEFER.
> >
> > That has the effect of hiding all the deferrals up until just before
> > launching into userspace, which should then acomplish two things -
> > firstly, getting rid of the rather useless deferred messages up to
> > that point, and secondly printing the reason why the remaining
> > deferrals are happening.
> >
> > That should be a small number of new lines plus a one-line change
> > in subsystems and drivers.
>
> Apart from the extra round we probably can't get rid of, that sounds
> OK to me.
Yes, it means a little longer to boot, but if there's nothing pending
this _should_ only be microseconds - it should be nothing more than
setting the flag, possibly taking and releasing a lock, and checking
that the deferred probe list is empty.
Of course, if the deferred probe list isn't empty, then there'll be
more expense - but I'm willing to bet that for those developers with
serial console enabled, the kernel boot will be faster overall due
to the reduced number of characters printed during the boot.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Geert Uytterhoeven @ 2015-10-19 16:21 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Tomeu Vizoso, Mark Brown, Greg Kroah-Hartman, Rob Herring,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood
In-Reply-To: <20151019153548.GM32532@n2100.arm.linux.org.uk>
Hi Russell,
On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>> > What you can do is print those devices which have failed to probe at
>> > late_initcall() time - possibly augmenting that with reports from
>> > subsystems showing what resources are not available, but that's only
>> > a guide, because of the "it might or might not be in a kernel module"
>> > problem.
>>
>> Well, adding those reports would give you a changelog similar to the
>> one in this series...
>
> I'm not sure about that, because what I was thinking of is adding
> a flag which would be set at late_initcall() time prior to running
> a final round of deferred device probing.
Which round is the final round?
That's the one which didn't manage to bind any new devices to drivers,
which is something you only know _after_ the round has been run.
So I think we need one extra round to handle this.
> This flag would then be used in a deferred_warn() printk function
> which would normally be silent, but when this flag is set, it would
> print the reason for the deferral - and this would replace (or be
> added) to the subsystems and drivers which return -EPROBE_DEFER.
>
> That has the effect of hiding all the deferrals up until just before
> launching into userspace, which should then acomplish two things -
> firstly, getting rid of the rather useless deferred messages up to
> that point, and secondly printing the reason why the remaining
> deferrals are happening.
>
> That should be a small number of new lines plus a one-line change
> in subsystems and drivers.
Apart from the extra round we probably can't get rid of, that sounds OK to me.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Mark Brown @ 2015-10-19 16:04 UTC (permalink / raw)
To: David Woodhouse
Cc: Rob Herring, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445268580.53393.183.camel@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]
On Mon, Oct 19, 2015 at 04:29:40PM +0100, David Woodhouse 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.
I'd expect that to be the norm rather than the exception.
> > 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 :)
Given the above I'm not even sure it's legacy code, it's just as likely
we're going to get some parallel ACPI code added to the subsystems for
parsing their bindings.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Rob Herring @ 2015-10-19 15:58 UTC (permalink / raw)
To: David Woodhouse
Cc: Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445268580.53393.183.camel@infradead.org>
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
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 15:43 UTC (permalink / raw)
To: David Woodhouse
Cc: Mark Brown, Rob Herring, Greg Kroah-Hartman, Tomeu Vizoso,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445268580.53393.183.camel@infradead.org>
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.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 15:35 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Mark Brown, Greg Kroah-Hartman, Rob Herring, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <CAAObsKDWM6A98tdKHqi4fe5DBf0BmB0JeghcZDWan_PYbyshag@mail.gmail.com>
On Mon, Oct 19, 2015 at 05:00:54PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 16:30, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > I typically see one or two, maybe five maximum on the platforms I have
> > here, but normally zero.
>
> Hmm, I have given a look at our lava farm and have seen 2 dozens as
> common (with multi_v7).
No, because the lava farms tend not to be public.
> > What you can do is print those devices which have failed to probe at
> > late_initcall() time - possibly augmenting that with reports from
> > subsystems showing what resources are not available, but that's only
> > a guide, because of the "it might or might not be in a kernel module"
> > problem.
>
> Well, adding those reports would give you a changelog similar to the
> one in this series...
I'm not sure about that, because what I was thinking of is adding
a flag which would be set at late_initcall() time prior to running
a final round of deferred device probing.
This flag would then be used in a deferred_warn() printk function
which would normally be silent, but when this flag is set, it would
print the reason for the deferral - and this would replace (or be
added) to the subsystems and drivers which return -EPROBE_DEFER.
That has the effect of hiding all the deferrals up until just before
launching into userspace, which should then acomplish two things -
firstly, getting rid of the rather useless deferred messages up to
that point, and secondly printing the reason why the remaining
deferrals are happening.
That should be a small number of new lines plus a one-line change
in subsystems and drivers.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: David Woodhouse @ 2015-10-19 15:29 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Herring, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <20151019145048.GI14956@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]
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
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Tomeu Vizoso @ 2015-10-19 15:00 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Mark Brown, Greg Kroah-Hartman, Rob Herring, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <20151019143045.GE32532@n2100.arm.linux.org.uk>
On 19 October 2015 at 16:30, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 19, 2015 at 04:10:56PM +0200, Tomeu Vizoso wrote:
>> On 19 October 2015 at 15:18, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
>> >> ... If a device is available and has
>> >> a compatible driver, but it cannot be probed because a dependency
>> >> isn't going to be available, that's an error and is going to cause
>> >> real-world problems unless the device is redundant. Currently we say
>> >> nothing because with deferred probe the probe callbacks are also part
>> >> of the mechanism that determines the dependency order.
>> >
>> > So what if device X depends on device Y, and we have a driver for
>> > device Y built-in to the kernel, but the driver for device X is a
>> > module?
>> >
>> > I don't see this being solvable in the way you describe above - it's
>> > going to identify X as being unable to be satisfied, and report it as
>> > an error - but it's not an error at all.
>>
>> It's going to probe Y at late_initcall, then probe X when its driver
>> is registered. No deferred probes nor messages about it.
>>
>> But if you meant to write the opposite case (X built-in and Y in a
>> module), then I have to ask you in what situation that would make
>> sense.
>
> I did mean the opposite way around. It may not make sense if you're
> targetting a single platform, but it may make sense in a single zImage
> kernel.
>
> Consider something like a single zImage kernel that is built with
> everything built-in to be able to boot and mount rootfs without
> initramfs support on both platform A and platform B. Both platforms
> share some hardware (eg, an I2C GPIO expander) which is built as a
> module. It is a resource provider. Platform B contains a driver
> which is required to boot on platform A, but not platform B (so the
> kernel has to have that driver built-in.) On platform B, there is
> a dependency to the I2C GPIO expander device.
I see, in this situation the person trying to find out why some device
hadn't probed would enable debug logging of failed probes and would
see one spurious message if there was a deferred probe because of the
module.
>> >> Having a specific switch for enabling deferred probe logging sounds
>> >> good, but there's going to be hundreds of spurious messages about
>> >> deferred probes that were just deferrals and only one of them is going
>> >> to be the actual error in which a device failed to find a dependency.
>> >
>> > Why would there be? Sounds like something's very wrong there.
>>
>> Sorry about that, I have checked that only now and I "only" get 39
>> deferred probe messages on exynos5250-snow.
>
> I typically see one or two, maybe five maximum on the platforms I have
> here, but normally zero.
Hmm, I have given a look at our lava farm and have seen 2 dozens as
common (with multi_v7).
>> > So, really, after boot and all appropriate modules have been loaded,
>> > you should end up with no deferred probes. Are you saying that you
>> > still have "hundreds" at that point? If you do, that sounds like
>> > there's something very wrong.
>>
>> I was talking about messages if we log each -EPROBE_DEFER, not devices
>> that remain to be probed. The point being that right now we don't have
>> a way to know if we are deferring because the dependency will be
>> around later, or if we have a problem and the dependency isn't going
>> to be there at all.
>
> What's the difference between a dependency which isn't around because
> the driver is not built into the kernel but is available as a module,
> and a dependency that isn't around because the module hasn't been
> loaded yet?
>
> How do you distinguish between those two scenarios? In the former
> scenario, the device will eventually come up when udev loads the
> module. In the latter case, it's a persistent failing case.
Agreed, but it's something that doesn't happen often and that's why
such messages would be at the debug level instead of being warns or
errors.
>> Agreed, with the note from above on why it would be better to only
>> print such a message only when the -EPROBE_DEFER is likely to be a
>> problem.
>
> ... and my argument is that there's _no way_ to know for certain which
> deferred probes will be a problem, and which won't. The only way to
> definitely know that is if you disable kernel modules, and require
> all drivers to be built into the kernel.
>
> What you can do is print those devices which have failed to probe at
> late_initcall() time - possibly augmenting that with reports from
> subsystems showing what resources are not available, but that's only
> a guide, because of the "it might or might not be in a kernel module"
> problem.
Well, adding those reports would give you a changelog similar to the
one in this series...
Thanks,
Tomeu
> --
> 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/
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Mark Brown @ 2015-10-19 14:50 UTC (permalink / raw)
To: David Woodhouse
Cc: Rob Herring, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445258870.53393.173.camel@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 14:30 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Mark Brown, Greg Kroah-Hartman, Rob Herring, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <CAAObsKDS_+AZ26P_fHc6pvAYrQotsuoO0DEGQUWC514t3DD_jQ@mail.gmail.com>
On Mon, Oct 19, 2015 at 04:10:56PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 15:18, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> >> ... If a device is available and has
> >> a compatible driver, but it cannot be probed because a dependency
> >> isn't going to be available, that's an error and is going to cause
> >> real-world problems unless the device is redundant. Currently we say
> >> nothing because with deferred probe the probe callbacks are also part
> >> of the mechanism that determines the dependency order.
> >
> > So what if device X depends on device Y, and we have a driver for
> > device Y built-in to the kernel, but the driver for device X is a
> > module?
> >
> > I don't see this being solvable in the way you describe above - it's
> > going to identify X as being unable to be satisfied, and report it as
> > an error - but it's not an error at all.
>
> It's going to probe Y at late_initcall, then probe X when its driver
> is registered. No deferred probes nor messages about it.
>
> But if you meant to write the opposite case (X built-in and Y in a
> module), then I have to ask you in what situation that would make
> sense.
I did mean the opposite way around. It may not make sense if you're
targetting a single platform, but it may make sense in a single zImage
kernel.
Consider something like a single zImage kernel that is built with
everything built-in to be able to boot and mount rootfs without
initramfs support on both platform A and platform B. Both platforms
share some hardware (eg, an I2C GPIO expander) which is built as a
module. It is a resource provider. Platform B contains a driver
which is required to boot on platform A, but not platform B (so the
kernel has to have that driver built-in.) On platform B, there is
a dependency to the I2C GPIO expander device.
> >> Having a specific switch for enabling deferred probe logging sounds
> >> good, but there's going to be hundreds of spurious messages about
> >> deferred probes that were just deferrals and only one of them is going
> >> to be the actual error in which a device failed to find a dependency.
> >
> > Why would there be? Sounds like something's very wrong there.
>
> Sorry about that, I have checked that only now and I "only" get 39
> deferred probe messages on exynos5250-snow.
I typically see one or two, maybe five maximum on the platforms I have
here, but normally zero.
> > So, really, after boot and all appropriate modules have been loaded,
> > you should end up with no deferred probes. Are you saying that you
> > still have "hundreds" at that point? If you do, that sounds like
> > there's something very wrong.
>
> I was talking about messages if we log each -EPROBE_DEFER, not devices
> that remain to be probed. The point being that right now we don't have
> a way to know if we are deferring because the dependency will be
> around later, or if we have a problem and the dependency isn't going
> to be there at all.
What's the difference between a dependency which isn't around because
the driver is not built into the kernel but is available as a module,
and a dependency that isn't around because the module hasn't been
loaded yet?
How do you distinguish between those two scenarios? In the former
scenario, the device will eventually come up when udev loads the
module. In the latter case, it's a persistent failing case.
> Agreed, with the note from above on why it would be better to only
> print such a message only when the -EPROBE_DEFER is likely to be a
> problem.
... and my argument is that there's _no way_ to know for certain which
deferred probes will be a problem, and which won't. The only way to
definitely know that is if you disable kernel modules, and require
all drivers to be built into the kernel.
What you can do is print those devices which have failed to probe at
late_initcall() time - possibly augmenting that with reports from
subsystems showing what resources are not available, but that's only
a guide, because of the "it might or might not be in a kernel module"
problem.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Tomeu Vizoso @ 2015-10-19 14:10 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Mark Brown, Greg Kroah-Hartman, Rob Herring, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <20151019131821.GA32532-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
On 19 October 2015 at 15:18, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
>> ... If a device is available and has
>> a compatible driver, but it cannot be probed because a dependency
>> isn't going to be available, that's an error and is going to cause
>> real-world problems unless the device is redundant. Currently we say
>> nothing because with deferred probe the probe callbacks are also part
>> of the mechanism that determines the dependency order.
>
> So what if device X depends on device Y, and we have a driver for
> device Y built-in to the kernel, but the driver for device X is a
> module?
>
> I don't see this being solvable in the way you describe above - it's
> going to identify X as being unable to be satisfied, and report it as
> an error - but it's not an error at all.
It's going to probe Y at late_initcall, then probe X when its driver
is registered. No deferred probes nor messages about it.
But if you meant to write the opposite case (X built-in and Y in a
module), then I have to ask you in what situation that would make
sense.
>> Having a specific switch for enabling deferred probe logging sounds
>> good, but there's going to be hundreds of spurious messages about
>> deferred probes that were just deferrals and only one of them is going
>> to be the actual error in which a device failed to find a dependency.
>
> Why would there be? Sounds like something's very wrong there.
Sorry about that, I have checked that only now and I "only" get 39
deferred probe messages on exynos5250-snow.
> You should only get deferred probes for devices which are declared to
> be present, but their resources have not yet been satisfied. It
> doesn't change anything if you have a kernel with lots of device drivers
> or just the device drivers you need - the device drivers you don't need
> do not contribute to the deferred probing in any way.
I don't think that the number of registered drivers affects the number
of probes that get deferred (but I'm not sure why you mention that).
> So, really, after boot and all appropriate modules have been loaded,
> you should end up with no deferred probes. Are you saying that you
> still have "hundreds" at that point? If you do, that sounds like
> there's something very wrong.
I was talking about messages if we log each -EPROBE_DEFER, not devices
that remain to be probed. The point being that right now we don't have
a way to know if we are deferring because the dependency will be
around later, or if we have a problem and the dependency isn't going
to be there at all.
If we had a way to enable printing the cause of each -EPROBE_DEFER,
right now that would print 39 messages of this board that are only due
to ordering. The actual issue would be printed in exactly the same way
somewhere in the middle.
>> 3) Regarding total boot time, I don't expect this series to make much
>> of a difference because though we would save a lot of matching and
>> querying for resources, that's little time compared with how long we
>> wait for hardware to react during probing. Async probing is more
>> likely to help with drivers that take a long time to probe.
>
> For me, on my fastest ARM board, though running serial console:
>
> [ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
>
> There's a couple of delays in there, but they're not down to deferred
> probing. The biggest one is serial console startup (due to the time
> it takes to write out the initial kernel messages):
>
> [ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
> [ 0.944124] console [ttyS0] enabled
>
> and DSA switch initialisation:
>
> [ 1.530655] libphy: dsa slave smi: probed
> [ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
>
> I'm not sure what causes that, but at a guess it's having to talk to the
> DSA switch over the MDIO bus via several layers of indirect accesses.
> Of course, serial console adds to the boot time significantly anyway,
> especially at the "standard" kernel logging level.
Yes, I don't think it makes any sense to measure boot times with the
serial console on, because it's not comparable to production and
because printing an additional line during boot affects significantly
the times.
To be clear, I was saying that this series should NOT affect total
boot times much.
>> One more thing about the breakage we have seen so far is that it's
>> generally caused by implicit dependencies and hunting those is
>> probably the second biggest timesink of the linux embedded developer
>> after failed probes.
>
> ... which is generally caused by the crappy code which the average
> embedded Linux developer creates, particularly with the crappy error
> messages they like creating. For the most part, they _might_ as well
> just print "Error!\n" and be done with it, for all the use they are.
> When creating an error print, your average embedded Linux developer
> fails to print the _reason_ why something failed, which makes debugging
> it much harder.
>
> The first thing I do when I touch code that needs this kind of debugging
> is to go through and add printing of the error code. That normally lets
> me quickly narrow down what's failed.
>
> If embedded Linux developers are struggling with this, they only have
> themselves to blame.
>
> In the case of deferred probing, what _may_ help is if we got rid of the
> core code printing that driver X requested deferred probing, instead
> moving the responsibility to report this onto the driver or subsystem.
> Resource claiming generally has the struct device, and can use dev_warn()
> to report which device is being probed, along with which resource is
> not yet available.
Agreed, with the note from above on why it would be better to only
print such a message only when the -EPROBE_DEFER is likely to be a
problem.
> This debug problem is solvable without needing to resort to complex
> probing solutions.
If you really think anything in this series is complex, you should
look at the other ones that tried to accomplish the same!
Thanks,
Tomeu
> --
> 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/
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 13:18 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Mark Brown, Greg Kroah-Hartman, Rob Herring, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <CAAObsKB2BUZ-smid45wOdAQw6h2yNqCydk+azAFNk69ewHJtZQ@mail.gmail.com>
On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> ... If a device is available and has
> a compatible driver, but it cannot be probed because a dependency
> isn't going to be available, that's an error and is going to cause
> real-world problems unless the device is redundant. Currently we say
> nothing because with deferred probe the probe callbacks are also part
> of the mechanism that determines the dependency order.
So what if device X depends on device Y, and we have a driver for
device Y built-in to the kernel, but the driver for device X is a
module?
I don't see this being solvable in the way you describe above - it's
going to identify X as being unable to be satisfied, and report it as
an error - but it's not an error at all.
> Having a specific switch for enabling deferred probe logging sounds
> good, but there's going to be hundreds of spurious messages about
> deferred probes that were just deferrals and only one of them is going
> to be the actual error in which a device failed to find a dependency.
Why would there be? Sounds like something's very wrong there.
You should only get deferred probes for devices which are declared to
be present, but their resources have not yet been satisfied. It
doesn't change anything if you have a kernel with lots of device drivers
or just the device drivers you need - the device drivers you don't need
do not contribute to the deferred probing in any way.
So, really, after boot and all appropriate modules have been loaded,
you should end up with no deferred probes. Are you saying that you
still have "hundreds" at that point? If you do, that sounds like
there's something very wrong.
> 3) Regarding total boot time, I don't expect this series to make much
> of a difference because though we would save a lot of matching and
> querying for resources, that's little time compared with how long we
> wait for hardware to react during probing. Async probing is more
> likely to help with drivers that take a long time to probe.
For me, on my fastest ARM board, though running serial console:
[ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
There's a couple of delays in there, but they're not down to deferred
probing. The biggest one is serial console startup (due to the time
it takes to write out the initial kernel messages):
[ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
[ 0.944124] console [ttyS0] enabled
and DSA switch initialisation:
[ 1.530655] libphy: dsa slave smi: probed
[ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
I'm not sure what causes that, but at a guess it's having to talk to the
DSA switch over the MDIO bus via several layers of indirect accesses.
Of course, serial console adds to the boot time significantly anyway,
especially at the "standard" kernel logging level.
> One more thing about the breakage we have seen so far is that it's
> generally caused by implicit dependencies and hunting those is
> probably the second biggest timesink of the linux embedded developer
> after failed probes.
... which is generally caused by the crappy code which the average
embedded Linux developer creates, particularly with the crappy error
messages they like creating. For the most part, they _might_ as well
just print "Error!\n" and be done with it, for all the use they are.
When creating an error print, your average embedded Linux developer
fails to print the _reason_ why something failed, which makes debugging
it much harder.
The first thing I do when I touch code that needs this kind of debugging
is to go through and add printing of the error code. That normally lets
me quickly narrow down what's failed.
If embedded Linux developers are struggling with this, they only have
themselves to blame.
In the case of deferred probing, what _may_ help is if we got rid of the
core code printing that driver X requested deferred probing, instead
moving the responsibility to report this onto the driver or subsystem.
Resource claiming generally has the struct device, and can use dev_warn()
to report which device is being probed, along with which resource is
not yet available.
This debug problem is solvable without needing to resort to complex
probing solutions.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: David Woodhouse @ 2015-10-19 12:47 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <CAL_JsqKehmSobpbdeXGo=-1pAbtrhCHovyyCWya_MNOhGCi8CQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
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
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Rob Herring @ 2015-10-19 12:35 UTC (permalink / raw)
To: David Woodhouse
Cc: Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445247881.53393.146.camel@infradead.org>
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
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Tomeu Vizoso @ 2015-10-19 12:34 UTC (permalink / raw)
To: Mark Brown
Cc: Greg Kroah-Hartman, Rob Herring, Russell King, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Liam Girdwood,
Felipe Balbi
In-Reply-To: <20151018195330.GB14956@sirena.org.uk>
On 18 October 2015 at 21:53, Mark Brown <broonie@kernel.org> 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...
Hi all,
hope you don't mind I summarize the points taken instead of replying
to the individual emails. I tried to address all the concerns that
have been raised again in the cover letter, but I guess I did a bad
job at explaining myself, so here's another (more in-depth) go at it.
1) About the sprinkling of calls, everybody agreed it's a bad smell
from the start, but the intention is to modify the behaviour of the
already-DT-specific part of each subsystem without duplicating code.
A way to avoid the sprinkling would be to move the storage and lookup
of resources to the core (using classes and their list of devices to
replace the likes of __of_usb_find_phy). I also like Mark's idea of
calling of_device_probe from of_parse_phandle, which would be much
less invasive but I'm not sure if it would be right to call that
function in all the current cases in which of_parse_phandle is called.
2) About the goal of the series, what matters to my employer is that
once a device defers its probe it's only going to be reprobed in
late_initcall, after all the devices have been tentatively probed
once. In the practice this means that devices get probed in a
dependency order in which first go devices without dependencies then
go up the tree until the leave devices (which tend to be the ones with
effects visible to the user).
This series changes to another dependency order in which when a leaf
node gets probed, it recursively "pulls" its dependencies. This way we
stop massively delaying the probing of the display devices and vendors
can stop carrying sizeable hacks in their trees which just further
reduce the incentive to upstream.
The above is what funds this work, but in my personal opinion the
biggest advantage of this work is that it makes development on
embedded platforms more efficient because right now we don't have a
way of telling if a device deferred its probe because of an ordering
issue, or because there's a problem. If a device is available and has
a compatible driver, but it cannot be probed because a dependency
isn't going to be available, that's an error and is going to cause
real-world problems unless the device is redundant. Currently we say
nothing because with deferred probe the probe callbacks are also part
of the mechanism that determines the dependency order. I have wasted
countless hours hunting for the reason why a device didn't probe and I
have heard the same several times from others.
Having a specific switch for enabling deferred probe logging sounds
good, but there's going to be hundreds of spurious messages about
deferred probes that were just deferrals and only one of them is going
to be the actual error in which a device failed to find a dependency.
3) Regarding total boot time, I don't expect this series to make much
of a difference because though we would save a lot of matching and
querying for resources, that's little time compared with how long we
wait for hardware to react during probing. Async probing is more
likely to help with drivers that take a long time to probe.
4) About the breakage we have seen, that's not caused so far by
probing devices on-demand but by delaying probes until all built-in
drivers have been registered. The latter isn't strictly needed for
on-demand probing but without it most of the benefits are lost because
probes of dependencies are going to be deferred because the drivers
aren't there yet. We could avoid that by registering drivers also
on-demand but we would need to make the matching information available
beforehand, which is a massive change in itself. This should speed up
boot some, and also cause leaf devices to be up earlier.
One more thing about the breakage we have seen so far is that it's
generally caused by implicit dependencies and hunting those is
probably the second biggest timesink of the linux embedded developer
after failed probes. We depend on hacks such as link order, node order
in the DT, initcall gerrymandering and a blind hope in things that
started some time ago to have finished by now. And those implicit
dependencies are often left completely undocumented. This is really
fragile and breaks often when changing something unrelated such as
when adding another revision of a board or soc and a dependency starts
deferring its probe or is delayed because of something else. Also
breaks with async probing.
Delayed probes can be reverted by disabling a Kconfig, so we can fix
those issues in an ordered manner as time allows (we could disable it
by default now and add CI jobs with that enabled during a transitory
period).
Back when I made the series FW-independent with fwnode additions I
felt in my interaction with the ACPI folks that there's a bit of a
chasm in this issue between embedded and non-embedded people. This
could be because with ACPI most of the low-level hw elements such as
clocks, regulators, gpios and pins are hidden from the kernel and are
already ready when we start probing devices. With DT, the kernel has
to initialize all those and only then it can initialize the higher
level devices that depend on them. This means lots more of devices and
dependencies and thus we feel more acutely the shortcomings of the
current device framework at the scale we are using it today.
I think that having all dependencies be explicit and represented in
the device-driver model, along with a more advanced method of ordering
probes is something that would be good to have at this moment, even if
it won't benefit all users of the kernel.
Thanks,
Tomeu
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Mark Brown @ 2015-10-19 11:02 UTC (permalink / raw)
To: David Woodhouse
Cc: Greg Kroah-Hartman, Tomeu Vizoso, Rob Herring, Russell King,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445247881.53393.146.camel@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Heiko Stübner @ 2015-10-19 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-1-git-send-email-boris.brezillon@free-electrons.com>
Hi Thierry,
Am Montag, 21. September 2015, 11:33:17 schrieb Boris Brezillon:
> Hello,
>
> This series adds support for atomic PWM update, or IOW, the capability
> to update all the parameters of a PWM device (enabled/disabled, period,
> duty and polarity) in one go.
is anything more blocking this series? It's now sitting on the lists for
nearly a month and everybody seems happy with it, so it would be really nice
to have in mainline :-) .
Especially as this also makes it possible for Rockchip Chromebooks to actually
control the logic-regulator that is implemented as pwm-regulator there.
Thanks
Heiko
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: Russell King - ARM Linux @ 2015-10-19 9:52 UTC (permalink / raw)
To: David Woodhouse
Cc: Mark Brown, Greg Kroah-Hartman, Tomeu Vizoso, Rob Herring,
Michael Turquette, Stephen Boyd, Vinod Koul, Dan Williams,
Linus Walleij, Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi
In-Reply-To: <1445247881.53393.146.camel@infradead.org>
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.
^ permalink raw reply
* Re: [GIT PULL] On-demand device probing
From: David Woodhouse @ 2015-10-19 9:44 UTC (permalink / raw)
To: Mark Brown, Greg Kroah-Hartman
Cc: Tomeu Vizoso, Rob Herring, Russell King, Michael Turquette,
Stephen Boyd, Vinod Koul, Dan Williams, Linus Walleij,
Alexandre Courbot, Thierry Reding, David Airlie,
Terje Bergström, Stephen Warren, Wolfram Sang, Frank Rowand,
Grant Likely, Kishon Vijay Abraham I, Sebastian Reichel,
Dmitry Eremin-Solenikov, Liam Girdwood, Felipe Balbi, Jingoo Han
In-Reply-To: <20151018195330.GB14956@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]
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
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox