linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@baylibre.com>
To: Saravana Kannan <saravanak@google.com>
Cc: Russell King <linux@armlinux.org.uk>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Tony Lindgren <tony@atomide.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	kernel-team@android.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-oxnas@groups.io,
	linux-renesas-soc@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v4 1/2] drivers: bus: simple-pm-bus: Add support for probing simple bus only devices
Date: Tue, 01 Feb 2022 10:36:11 -0800	[thread overview]
Message-ID: <7hee4mmo2s.fsf@baylibre.com> (raw)
In-Reply-To: <CAGETcx_YEUxEBSBnzFaBxW=9=jO6BO0GuThaMGF+JPkDeC-ivw@mail.gmail.com>

Saravana Kannan <saravanak@google.com> writes:

> On Mon, Jan 31, 2022 at 7:18 PM Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> Hi Saravana,
>>
>> Saravana Kannan <saravanak@google.com> writes:
>>
>> > fw_devlink could end up creating device links for bus only devices.
>> > However, bus only devices don't get probed and can block probe() or
>> > sync_state() [1] call backs of other devices. To avoid this, probe these
>> > devices using the simple-pm-bus driver.
>> >
>> > However, there are instances of devices that are not simple buses (they get
>> > probed by their specific drivers) that also list the "simple-bus" (or other
>> > bus only compatible strings) in their compatible property to automatically
>> > populate their child devices. We still want these devices to get probed by
>> > their specific drivers. So, we make sure this driver only probes devices
>> > that are only buses.
>> >
>> > [1] - https://lore.kernel.org/lkml/CAPDyKFo9Bxremkb1dDrr4OcXSpE0keVze94Cm=zrkOVxHHxBmQ@mail.gmail.com/
>> > Fixes: c442a0d18744 ("driver core: Set fw_devlink to "permissive" behavior by default")
>> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>> > Tested-by: Saravana Kannan <saravanak@google.com>
>> > Tested-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> This patch landed in stable/linux-5.10.y as commit d5f13bbb5104 and it
>> broke suspend/resume on at least one TI AM335x board I'm testing on:
>> upstream dts: arch/arm/boot/dts/am335x-icev2.dts, upstream defconfig:
>> arch/arm/configs/omap2plus_defconfig.
>>
>> Bisecting between vanilla v5.10 (good) and stable/linux-5.10.y (bad)
>> pointed me to this patch, and I confirmed that reverting just this patch
>> on top of stable/linux-5.10.y makes it work again.
>>
>> Also interesting, this same platform works fine on vanilla v5.15, which
>> also includes this patch.  That suggests that either 1) this patch
>> should not have been backported to v5.10 stable or 2) there are some
>> other dependencies that are missing in v5.10.
>>
>> Since vanilla v5.10 works fine, I'm leaning towards (1), but if you have
>> any ideas for deps that need backporting, I'm happy to try.
>
> Oh wow! I didn't realize I made so many changes AFTER 5.10! Unless I'm
> doing something wrong with my git commands.
> $ git log v5.10..v5.15 --oneline -- drivers/of/property.c
> $ git log v5.10..v5.15 --oneline --author=saravanak -- drivers/base/
>
> If you don't think I got my git command completely wrong, yeah, way
> too many patches are missing on 5.10. I'd go with the option of
> dropping this patch on 5.10.

I agree.  Could you submit a revert for v5.10 stable?  As the patch
author, it's probably better if it comes from you.

>> I haven't debugged exactly where it's hanging yet, but, enabling
>> CONFIG_DEBUG_DRIVER=y, and suspending with "no_console_suspend" on the
>> command line, the last line before it hangs is:
>>
>>    [   28.129966] simple-pm-bus ocp: noirq power domain suspend
>>
>> Any ideas?
>
> I'd guess it's either a sync_state() happening too soon since some of
> the dependencies aren't tracked. Or some dependency cycle that'd be
> handled correctly if the rest of the patches were picked up. Yeah, a
> pretty broad/vague answer.

Heh, yeah, and also suggests that there's other gotchas hiding in other
dark corners, which also suggests that revert is the best path for
v5.10.

Thanks for the quick response,

Kevin

  reply	other threads:[~2022-02-01 18:36 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29  0:07 [PATCH v4 0/2] Fix simple-bus issues with fw_devlink Saravana Kannan
2021-09-29  0:07 ` [PATCH v4 1/2] drivers: bus: simple-pm-bus: Add support for probing simple bus only devices Saravana Kannan
2021-10-05 15:07   ` Geert Uytterhoeven
2021-11-10 12:16   ` Abel Vesa
2021-11-10 20:24     ` Saravana Kannan
2021-11-15 20:17       ` Saravana Kannan
2021-11-16  5:19         ` Tony Lindgren
2021-11-16 13:59         ` Abel Vesa
2021-11-16 15:11           ` Abel Vesa
2021-11-19 16:16             ` Abel Vesa
2021-11-24  1:07               ` Saravana Kannan
2022-02-01  3:18   ` Kevin Hilman
2022-02-01  5:30     ` Saravana Kannan
2022-02-01 18:36       ` Kevin Hilman [this message]
2022-02-02 19:58         ` Kevin Hilman
2021-09-29  0:07 ` [PATCH v4 2/2] drivers: bus: Delete CONFIG_SIMPLE_PM_BUS Saravana Kannan
2021-10-05 15:11   ` Geert Uytterhoeven
2021-10-21 11:21   ` Amit Pundir
2021-10-21 12:37     ` Tony Lindgren
2021-10-21 23:42     ` Saravana Kannan
2021-10-22 16:59       ` Amit Pundir
2021-10-26  0:30         ` Saravana Kannan
2021-10-26 17:49           ` Amit Pundir
2021-09-30  2:50 ` [PATCH v4 0/2] Fix simple-bus issues with fw_devlink Damien Le Moal
2021-10-06  5:29 ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7hee4mmo2s.fsf@baylibre.com \
    --to=khilman@baylibre.com \
    --cc=catalin.marinas@arm.com \
    --cc=damien.lemoal@wdc.com \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-oxnas@groups.io \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=magnus.damm@gmail.com \
    --cc=narmstrong@baylibre.com \
    --cc=robh+dt@kernel.org \
    --cc=saravanak@google.com \
    --cc=tony@atomide.com \
    --cc=ulf.hansson@linaro.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).