Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Andrzej Hajda <a.hajda@samsung.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jonas Karlman <jonas@kwiboo.se>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	"open list:DRM DRIVERS" <dri-devel@lists.freedesktop.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH v6 2/4] driver core: add deferring probe reason to devices_deferred property
Date: Thu, 2 Jul 2020 08:57:55 +0200
Message-ID: <e6057292-39de-831c-0b8d-b3f0b66937dc@samsung.com> (raw)
In-Reply-To: <CAKdAkRRLBLCLGH2qhEjaVnt8wNjoyGAfQimNWHZUvzx2m6Mwng@mail.gmail.com>


On 30.06.2020 20:00, Dmitry Torokhov wrote:
> On Tue, Jun 30, 2020 at 8:42 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>
>> On 30.06.2020 10:59, Grygorii Strashko wrote:
>>> Hi
>>>
>>> On 29/06/2020 14:28, Andrzej Hajda wrote:
>>>> Hi Grygorii,
>>>>
>>>> (...)
>>>>
>>>>>>     /*
>>>>>>      * deferred_devs_show() - Show the devices in the deferred probe
>>>>>> pending list.
>>>>>>      */
>>>>>> @@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s,
>>>>>> void *data)
>>>>>>         mutex_lock(&deferred_probe_mutex);
>>>>>>           list_for_each_entry(curr, &deferred_probe_pending_list,
>>>>>> deferred_probe)
>>>>>> -        seq_printf(s, "%s\n", dev_name(curr->device));
>>>>>> +        seq_printf(s, "%s\t%s", dev_name(curr->device),
>>>>>> + curr->device->p->deferred_probe_reason ?: "\n");
>>>>>>           mutex_unlock(&deferred_probe_mutex);
>>>>>>
>>>>> Sry, may be i missing smth, but shouldn't it be optional
>>>>> (CONFIG_DEBUG_FS is probably too generic).
>>>>>
>>>> I am not sure what exactly are you referring to, but this patch does not
>>>> add new property, it just extends functionality of existing one.
>>> Sry, needed to be more specific.
>>>
>>> You've added  device_set_deferred_probe_reson(dev, &vaf);
>>> which expected to be used on every EPROBE_DEFER in dev_err_probe() in
>>> combination with
>>>
>>> +       } else {
>>> +               device_set_deferred_probe_reson(dev, &vaf);
>>>                  dev_dbg(dev, "error %d: %pV", err, &vaf);
>>>
>>> ^^ dev_dbg() does not add any runtime overhead during boot unless enabled
>>> +       }
>>>
>>> But:
>>>
>>> +void device_set_deferred_probe_reson(const struct device *dev, struct
>>> va_format *vaf)
>>> +{
>>> +       const char *drv = dev_driver_string(dev);
>>> +
>>> +       mutex_lock(&deferred_probe_mutex);
>>> +
>>> +       kfree(dev->p->deferred_probe_reason);
>>> +       dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s:
>>> %pV", drv, vaf);
>>> +
>>> +       mutex_unlock(&deferred_probe_mutex);
>>> +}
>>>
>>> ^^ Adds locking, kfree() and kasprintf() for every deferred probe
>>> during boot and can't be disabled.
>>>
>>> Right?
>>
>> Right, but usually the burden should be insignificant in comparison to
>> probe time, so I do not think it is worth optimizing.
> I do not think this is going to take. You are suggesting that we
> modify pretty much every driver to supply this deferral reason, and I
> doubt it will happen. Can we put this burden on providers that raise
> the deferral?


I wouldn't say they raise the deferral, they just inform resource is not 
yet available. Only device driver, and only in its probe function can 
"raise the deferral".


>   I.e. majority of code are using devm API now, so we most
> likely know the device for which deferral is being raised. We can have
> a list of deferral reasons and their devices and when in device code
> once probe is done we could try reconciling it with the deferred
> devicelist, and this would mean you only need to implement this in
> gpiolib, regulator core, clocks core, etc.


This patchset tries to solve simple issue - replace multiple lines of 
code present in multiple probe functions (additionally fixing lot of 
them) with single call and then enhance it little bit, nothing more.

What you are proposing is blurry at the moment for me, provider does not 
know if consumer want to defer,  or will continue working without 
missing resource, moreover some consumers can acquire resources after 
probe - again no probe deferral. Even if it will be done (it can be, for 
example by creating probe version of all resource get functions), it 
will require much more changes but finally it will look like:

res = devm_get_resource_from_probe(....)

if (IS_ERR(res))

     return PTR_ERR(res);

vs:

res = devm_get_resource(...)

if (IS_ERR(res))

     return dev_err_probe(dev, PTR_ERR(res), ...);


Regards

Andrzej


>
> Thanks.
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200626100108eucas1p2c6d68625f3755a467d7316dd27704f7c@eucas1p2.samsung.com>
2020-06-26 10:00 ` [PATCH v6 0/4] driver core: add probe error check helper Andrzej Hajda
     [not found]   ` <CGME20200626100109eucas1p25331652d017cd17d21c0ae60541d1f73@eucas1p2.samsung.com>
2020-06-26 10:01     ` [PATCH v6 1/4] driver core: add device probe log helper Andrzej Hajda
2020-06-26 15:41       ` Rafael J. Wysocki
2020-06-26 16:22       ` Sam Ravnborg
2020-07-01  9:29       ` Grygorii Strashko
     [not found]   ` <CGME20200626100110eucas1p2c5b91f2c98a5c6e5739f5af3207d192e@eucas1p2.samsung.com>
2020-06-26 10:01     ` [PATCH v6 2/4] driver core: add deferring probe reason to devices_deferred property Andrzej Hajda
2020-06-26 15:43       ` Rafael J. Wysocki
2020-06-26 17:11       ` Grygorii Strashko
2020-06-29 11:28         ` Andrzej Hajda
2020-06-30  8:59           ` Grygorii Strashko
2020-06-30 15:41             ` Andrzej Hajda
2020-06-30 18:00               ` Dmitry Torokhov
2020-07-02  6:57                 ` Andrzej Hajda [this message]
2020-07-07  4:14                   ` Dmitry Torokhov
2020-07-10  7:42                     ` Andrzej Hajda
2020-07-10 11:07                       ` Mark Brown
     [not found]   ` <CGME20200626100110eucas1p24327c924dada0c2e86ecf0ab5b5af571@eucas1p2.samsung.com>
2020-06-26 10:01     ` [PATCH v6 3/4] drm/bridge/sii8620: fix resource acquisition error handling Andrzej Hajda
2020-06-29  8:33       ` Neil Armstrong
     [not found]   ` <CGME20200626100111eucas1p18e175e6c77af483bd80fb90c171b05db@eucas1p1.samsung.com>
2020-06-26 10:01     ` [PATCH v6 4/4] drm/bridge: lvds-codec: simplify " Andrzej Hajda
2020-06-29  8:33       ` Neil Armstrong

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=e6057292-39de-831c-0b8d-b3f0b66937dc@samsung.com \
    --to=a.hajda@samsung.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=grygorii.strashko@ti.com \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=narmstrong@baylibre.com \
    --cc=rafael@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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git